Skip to content

Simplify consumption with a generalized StyleProvider#89

Draft
rolint wants to merge 2 commits into
HCL-TECH-SOFTWARE:mainfrom
rolint:generalized-style-provider
Draft

Simplify consumption with a generalized StyleProvider#89
rolint wants to merge 2 commits into
HCL-TECH-SOFTWARE:mainfrom
rolint:generalized-style-provider

Conversation

@rolint
Copy link
Copy Markdown
Contributor

@rolint rolint commented Jan 10, 2025 •
edited
Loading

work in progress

The expected outcome is an abstraction of the mechanics of styling an app (or an isolated component) with "Enchanted", with a generalized <StyleProvider>.

The StyleProvider also has the following "smarts":

  • direction fallback: look for the dir attribute of the <html> element, and behave accordingly
  • nonce fallback: look for a global styleNonce variable, for StrictCSP

For example:

Preamble:

import React from 'react';
import StyleProvider from '@hcl-software/enchanted-react-components/dist/StyleProvider ';
import MyRootComponent from '...';

Simplest:

export const MyEnchantedApp = () => {
  return (
    <StyleProvider>
      <MyRootComponent >
    </StyleProvider>
  )
}

Most complex:

export const MyEnchantedApp = () => {
 const themeOverrides = {
   ...
 };
  return (
    <StyleProvider 
      direction="rtl" 
      mode="LightCoolGrey" 
      nonce="abc123" 
      cssBaseline="scoped" 
      themeOverrides={themeOverrides}
    >
      <MyRootComponent >
    </StyleProvider>
  )
}

rolint
rolint commented Jan 10, 2025
View reviewed changes
Comment thread src/StyleProvider/StyleProvider.tsx

// Extend the Window interface to include styleNonce
// TODO - is this really the way to access a global with TypeScript??
declare global {
Copy link
Copy Markdown
Contributor Author

@rolint rolint Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what Copilot advised me to do, but I'm suspicious

Copy link
Copy Markdown
Collaborator

@HanjoHagemeierHCL HanjoHagemeierHCL Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Het @rolint, I think that should not work. If I understood this docu correct. It looks like the the window object has no styleNonce attribute. Where does the idea come from that the window object is providing the a styleNonce attribute?

Copy link
Copy Markdown
Contributor Author

@rolint rolint Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from a discussion we had (internally) about how to give a consuming application the option to set a global nonce value specifically for styles.

I do not know what the correct technique is for TypeScript to be happy with a global that we invented and is therefore not part of the official definition of the global window object.

Copy link
Copy Markdown
Collaborator

@HanjoHagemeierHCL HanjoHagemeierHCL Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rolint, I think this following option would be better.

  1. Create a file custom.window.ts
  2. Add those content
    export interface CustomWindow extends Window { customAttribute: string; }
  3. Now you can import the CustomWindow in the StyleProvider like that
    import { CustomWindow } from './custom.window.ts';
  4. Than please cast the global window object with CustomWindow
    declare let window: CustomWindow;
  5. And now you can use the customAttribute in your code.
    window.customAttribute = 'works'; // SET
    const value = window.customAttribute; // GET

rolint
rolint commented Jan 10, 2025
View reviewed changes
Comment thread src/StyleProvider/StyleProvider.tsx

const StyleProvider: React.FC<StyleProviderProps> = (props: StyleProviderProps) => {
// look for <html dir="rtl">
const htmlPageDirection = document.documentElement.getAttribute('dir') === 'rtl' ? ThemeDirectionType.RTL : ThemeDirectionType.LTR;
Copy link
Copy Markdown
Contributor Author

@rolint rolint Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it considered an antipattern for a React component to reach outside to the document?

Copy link
Copy Markdown
Collaborator

@HanjoHagemeierHCL HanjoHagemeierHCL Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rolint, no I think not. That should be okay.

HanjoHagemeierHCL
HanjoHagemeierHCL requested changes Jan 13, 2025
View reviewed changes
Comment thread src/StyleProvider/StyleProvider.tsx

const StyleProvider: React.FC<StyleProviderProps> = (props: StyleProviderProps) => {
// look for <html dir="rtl">
const htmlPageDirection = document.documentElement.getAttribute('dir') === 'rtl' ? ThemeDirectionType.RTL : ThemeDirectionType.LTR;
Copy link
Copy Markdown
Collaborator

@HanjoHagemeierHCL HanjoHagemeierHCL Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rolint, no I think not. That should be okay.

Comment thread src/StyleProvider/StyleProvider.tsx

// Extend the Window interface to include styleNonce
// TODO - is this really the way to access a global with TypeScript??
declare global {
Copy link
Copy Markdown
Collaborator

@HanjoHagemeierHCL HanjoHagemeierHCL Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Het @rolint, I think that should not work. If I understood this docu correct. It looks like the the window object has no styleNonce attribute. Where does the idea come from that the window object is providing the a styleNonce attribute?

@HanjoHagemeierHCL HanjoHagemeierHCL self-requested a review January 13, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HanjoHagemeierHCL HanjoHagemeierHCL Awaiting requested review from HanjoHagemeierHCL

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rolint @HanjoHagemeierHCL @rlint