-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DON'T MERGE] [WNMGDS-2640 WNMGDS-2160] Info architecture update #2913
base: main
Are you sure you want to change the base?
Conversation
* shift content around to prioritize guidance, add resources section w/accordion. * add optional prop to hide link footer from Storybook examples in docs. * update storybook guidance component with new language. * add storybook icon to links, remove alt text (decorative images) * fix word breaking on inline code
…2909) * implement updated component maturity model component. * comment out design criteria - not tested. * add list class to ul wrapping incomplete checklist items * change alignment in progress status - needs start instead of center. * add spacing to top of maturity checklist section.
…/CMSgov/design-system into feature/info-architecture-update
remove sub-levels from ToC, fix spacing issues in both lists.
* fix typo in storybook guidance component interface * hardcode sketchlink for testing. need to revisit later. * add functionality to design resource link
@zarahzachz Looking awesome! |
|
@zarahzachz Oh, one more thing. The links under "Have ideas" are unchanged. They should be as listed in this image: |
@hi-its-hailey for the updated table of contents links, where do those links go? Specifically the "Ask a question" and "Start a contribution" |
@hi-its-hailey for the accordion style, i'm working a ticket now to only use one theme in our docs site documentation. i think this will impact the accordions used, so i might hold off on updating until after that work's done. i'm going to try implementing the styles from healthcare's accordion though (like in the mocks) |
Ask a question: Slack let's also keep "start a discussion on github" ! Thank you! |
* remove unneeded ds-base class. * hard-code core theme vars for docs site. * remove bespoke component css and data-theme attr from docs site. * update language around font family util class docs * import hgov accordion styles, props and vars to docs. * add hgov package to docs, update prop in mobile toc * copy hgov AccordionItem component props and styles into docs to prevent bringing in entire hgov DS * clean up scss imports - move accordionitem styles to its own file.
}} | ||
> | ||
|
||
## Component maturity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of baking "Component maturity" into the <MaturityChecklist>
component as an <h2>
, I'm adding it as a child of that component. This was the only way I could get our table of contents to pick up that this heading existed so it could be added there.
|
||
<DesignResourceLink /> | ||
|
||
### Development | ||
|
||
This component has analytics tracking available. Please see our developer documentation about [using analytics in the design system](/getting-started/for-developers/#analytics). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure where our analytics content should live, so I added it under the Development section
@@ -13,7 +13,7 @@ export const OneColumnPageLayout = () => { | |||
return ( | |||
<> | |||
<Header /> | |||
<main className="ds-base ds-l-container example-grid"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ds-base
was deprecated as part of the CSS reset work
</article> | ||
</section> | ||
</EmbeddedExample> | ||
|
||
<ThemeContent neverThemes={['medicare']}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mgov doesn't have a serif font, so hiding the example from their documentation
@@ -0,0 +1,9 @@ | |||
import { AccordionItem, MinusCircleIcon, PlusCircleIcon } from '@cmsgov/design-system'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied Hgov's prop overrides for AccordionItem into the docs site
ButtonMigrationTable: (props) => <ButtonMigrationTable theme={theme} {...props} />, | ||
ButtonVariationsTable: (props) => <ButtonVariationsTable theme={theme} {...props} />, | ||
code: CodeWithSyntaxHighlighting, | ||
ColorExampleList: (props) => <ColorExampleList theme={theme} {...props} />, | ||
ColorRamps, | ||
ComponentThemeOptions: (props) => <ComponentThemeOptions theme={theme} {...props} />, | ||
DesignResourceLink: (props) => ( | ||
<DesignResourceLink frontmatter={frontmatter} theme={theme} {...props} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frontmatter
needs to be passed to DesignResourceLink
so this component can access content that exists in the page's frontmatter (e.g., Sketch links)
Sketch symbol | ||
</a> | ||
) : ( | ||
'This theme does not have a Sketch symbol.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conditional is needed due to not defining Sketch symbols for CMSgov theme
@@ -73,7 +73,7 @@ const Layout = ({ | |||
const pageId = slug ? `page--${slug.replace('/', '_')}` : null; | |||
|
|||
return ( | |||
<div className="ds-base" data-theme={theme} id={pageId}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ds-base
was deprecated in CSS reset work
@@ -48,7 +48,7 @@ const PageHeader = ({ frontmatter = { title: '' }, theme }: PageHeaderProps) => | |||
{storyId && ( | |||
<a href={makeStorybookUrl(storyId, theme, 'docs')} className="c-page-header__link"> | |||
<img | |||
alt="Storybook logo" | |||
alt="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These images are decorative only - so including alt
text is just noise. Empty alt
indicates a decorative image
@@ -11,10 +11,10 @@ pre { | |||
} | |||
|
|||
code { | |||
display: inline-block; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this so the inline code wouldn't result in periods wrapping to the next line at certain breakpoints
<div className="c-navigation__switchers-wrapper ds-u-display--none ds-u-md-display--block"> | ||
<div | ||
className="c-navigation__switchers-wrapper ds-u-display--none ds-u-md-display--block" | ||
style={{ backgroundColor: getThemeColorValue(theme, 'primary') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does work, I promise. If you aren't seeing the correct background color reflected in the theme switcher, navigate to another page on the site and it should "wake up." This is related to a bug I mentioned in #2922
tokensInSketch: CheckStatus; | ||
children: string; | ||
|
||
items: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a reason why this needed to change. Was this leftover from a previous version of the code? It looks like this could be reverted without any negative effects and down below it the references can be reverted from things like props.items['forcedColors']
to props.forcedColors
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you'd be able to get rid of all the changes in the diff that just had to do with reformatting the <MaturityChecklist>
instances
This work is still in draft mode. The IA needs to be tested with users further and validated. |
DON'T MERGE!
We're waiting on user testing of this demo url before merging into
main
. (Source)WNMGDS-2640
WNMGDS-2160
Designs
Demo url