feat(odyssey-react): Text component refactor in Banner #1160
Conversation
There was a problem hiding this comment.
So this adds text styles using components but what about the existing global styles that are already on the page via storybook? I think it will make it difficult to ensure we've done our job refactoring here. Can we break apart those typography styles for storybook and apply them conditionally on a per component basis similar to what we had to do for reset styles?
| @@ -15,6 +15,7 @@ | |||
| margin-block-end: $spacing-xs-em; | |||
| margin-inline: 0; | |||
| color: $text-heading; | |||
| font-family: $body-font-family; | |||
There was a problem hiding this comment.
When the global styles are removed the font-family definition for h tags go with it. Adding it here ensures that the correct font-family is used.
It seemed correct to use it here because this is a composable component like text.
ghost
assigned 
@jeffbelser-okta I removed them locally, as I did the work but your point is well taken. Would you be open to just removing them altogether? At the very least the style that pertains to the global typography, we are working quickly to refactor? The global styles in question: https://codepen.io/arnoldsandoval-okta/pen/MWvGxjO?editors=0100 |
arnoldsandoval-okta
force-pushed
the
as/text-banner-refactor
branch
from
1dbdc8b
to
a260fd6
Compare
|
@arnoldsandoval-okta thanks for the response! I like idea to just roll thru these without having to split apart fine grain global styles for each story just to confirm it's working as we expect. As long as we don't reach the end, remove the sheet and find that nothing works I think this is a good approach just requires a bit more developer effort to QA cc @andrewberg-okta |
| @@ -1,5 +1,3 @@ | |||
| import "@okta/odyssey-react/dist/odyssey-deprecated-global.251d01.css"; | |||
|
|
|||
There was a problem hiding this comment.
I'm not sure we can remove this wholesale because it will affect the other components in the sb - did you mean to remove this from your commit?
There was a problem hiding this comment.
Apologies @jeffbelser-okta , I misunderstood our original interaction around this. I took it to mean that you were OK with us removing these styles instead of doing it locally on a one off basis.
I have no preference and have reverted the commit which removed these styles. Should be good to go!
…m storybook" This reverts commit 247e5f0.
* chore(odyssey-react): add text component to tag (#1169) * chore(odyssey-react): add text component to infobox (#1171) * feat(odyssey-react): Text component refactor in Banner (#1160) * feat(odyssey-react): refactor Banner to use Text component. Add default font family to Heading * chore(odyssey-react): add semantic tag back in for banner * chore(odyssey-storybook): remove deprecated global styles from storybook * Revert "chore(odyssey-storybook): remove deprecated global styles from storybook" This reverts commit 247e5f0. * Add documentation template (#1166) * docs: added documentation template for contributors * chore(odyssey-react): fix Form type annotation and improve Toast.Provider story (#1175) * chore(odyssey-react): fix type annotations for form * chore(odyssey-react): use odyssey form component in Toast.Provider story * chore(odyssey-react): use relative path for form import in toast story * chore(odyssey-react): add text component to tooltip (#1170) * chore(odyssey-react): add text component to tooltip * chore(odyssey-react): remove text css for tooltip * build: use conventional commits for changelog and lint * test(odyssey-react): add coverage for controlled Select * refactor(odyssey-react): use Text for List * feat: support async select options * feat: expose hidden choices reference * fix: lint * fix: remove added dep / prop * fix: updates per review feedback
No description provided.