Skip to content

Conversation

@castastrophe
Copy link
Contributor

@castastrophe castastrophe commented Apr 14, 2020

Goal 1: Fix any stories not currently updating

Goal 2: Add context support to the storybook instances.

The current approach of using the built-in storybook background color settings is putting the background color outside the iframe that contains the components. That means that the components can't detect the color or read the theme property being set. I removed that setting and instead added a theming function to the utilities in storybook. Not every component responds to the context so I left it as opt-in. You can invoke the contexts in the storybook instance by adding:

tools.context();

This will set background color and the theme custom property on the body tag of the iframe containing the web component:

    document.querySelector("body").style.backgroundColor = "#fff";
    document.querySelector("body").style.setProperty("--theme", "light");

Testing

Basic context
  1. Open the storybook instance.
  2. Navigate to the Accordion story.
  3. Set the context to complement.

Expect: The background color should change to #0477a4 and the accordion should respond to that context by invoking the saturated theme styles (white text, etc.)

Custom context
  1. Open the storybook instance.
  2. Navigate to the Accordion story.
  3. Set the context to custom.
  4. Choose a color from the color picker.
  5. Choose a context from the custom context dropdown.

Expect: The background color should change to the hex value in the color picker and the accordion should respond to that context by invoking the selected theme style.

Resetting the selections
  1. Run through the steps in the Custom context test case.
  2. To clear the custom selection, go to the first dropdown labeled Context and select lightest.
  3. The background should reset to the lightest color and the custom color and context dropdowns should disappear from the UI.

Goal 3: Fix imports for jump links to enable access to the nav and panel properties objects.

@castastrophe castastrophe requested a review from kylebuch8 April 14, 2020 14:39
@kylebuch8
Copy link
Contributor

@castastrophe I can't seem to remove the custom background color after I've chosen one. Is that a bug?

@castastrophe castastrophe marked this pull request as draft May 26, 2020 18:33
@castastrophe castastrophe marked this pull request as ready for review July 20, 2020 15:27
@castastrophe castastrophe added the demo Updating demo pages label Jul 20, 2020
@castastrophe
Copy link
Contributor Author

@kylebuch8 Updated this PR and added additional test cases to cover the resetting of the custom colors! Thanks!

@castastrophe castastrophe changed the title feat: Add theming support to storybook feat: Add context support to storybook Jul 20, 2020
@castastrophe castastrophe added the run e2e Trigger automated visual regression tests label Aug 25, 2020
@castastrophe
Copy link
Contributor Author

Bump @kylebuch8 or @starryeyez024. This adds the theming attribute support to storybook.

@castastrophe castastrophe removed the run e2e Trigger automated visual regression tests label Sep 3, 2020
@castastrophe castastrophe added the on hold wait to merge label Oct 20, 2020
@castastrophe
Copy link
Contributor Author

On holding pending base class PR updates

@castastrophe castastrophe added this to the 1.0 release milestone Nov 30, 2020
@castastrophe castastrophe added work in progress POC / Not ready for review and removed on hold wait to merge labels Nov 30, 2020
@castastrophe castastrophe changed the title feat: Add context support to storybook fix: Storybook bug fixes; jump links imports Dec 1, 2020
<link href="https://fonts.googleapis.com/css?family=Open+Sans&display=swap" rel="stylesheet">
<link rel="preconnect" href="https://fonts.gstatic.com">
<link
href="https://fonts.googleapis.com/css2?family=Overpass+Mono:wght@300;400;600;700&family=Overpass:ital,wght@0,100;0,200;0,300;0,400;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,600;1,700;1,800;1,900&family=Red+Hat+Display:ital,wght@0,400;0,500;0,700;0,900;1,400;1,500;1,700;1,900&family=Red+Hat+Text:ital,wght@0,400;0,500;0,700;1,400;1,500;1,700&display=swap"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@starryeyez024 There is an issue importing the fonts from Google isn't there? We're not referencing the Red Hat fonts in the project yet but if we do the weight mappings are off right?

Copy link
Member

@starryeyez024 starryeyez024 Dec 1, 2020

Choose a reason for hiding this comment

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

PFE can/should use the google font import method since its public, and the variables in the theme should match google fonts. https://github.com/patternfly/patternfly-elements/pull/1180/files#diff-72f58e02c1bef4a30c0860a93753d6c664bcc75326b9dc79911d410bf1c0d91bR9

Redhat-theme makes use of the font on static, and set different font weights:
https://gitlab.corp.redhat.com/uxdd/redhat-theme/-/merge_requests/103/diffs#8032f081053a55c41b3898c25e235f27f1daf4f6_43_43

Copy link
Member

Choose a reason for hiding this comment

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

It seems like this is totally fine as-is. The typography branch does the work of having the components load the RH fonts, so this just means storybook will be ready. And Overpass is working too currently 👍

@castastrophe castastrophe added ready: code review Ready for code review! and removed work in progress POC / Not ready for review labels Dec 1, 2020
@github-actions github-actions bot added docs Documentation updates functionality Functionality, typically pertaining to the JavaScript. generator Updates relating to the generator styles An issue or PR pertaining only to CSS/Sass tools Development and build tools labels Dec 1, 2020
Copy link
Contributor

@kylebuch8 kylebuch8 left a comment

Choose a reason for hiding this comment

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

Lives Generously, Though Miserly
🤷

@kylebuch8 kylebuch8 merged commit f50368b into master Dec 1, 2020
@kylebuch8 kylebuch8 deleted the feat-add-theming-storybook branch December 1, 2020 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

demo Updating demo pages docs Documentation updates functionality Functionality, typically pertaining to the JavaScript. generator Updates relating to the generator ready: code review Ready for code review! styles An issue or PR pertaining only to CSS/Sass tools Development and build tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants