Skip to content
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

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

zarahzachz
Copy link
Collaborator

@zarahzachz zarahzachz commented Jan 29, 2024

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

* 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.
@zarahzachz zarahzachz marked this pull request as draft January 29, 2024 21:28
@hi-its-hailey
Copy link
Collaborator

hi-its-hailey commented Jan 30, 2024

@zarahzachz Looking awesome!
With this accordian style, would it work to make the title background a different color then the page background? (white title background for the component maturity list, or grey for the code resources?) Or maybe an underline or outline? It looks like a strange amount of space when it just blends into the background
Screenshot 2024-01-30 at 11 38 43 AM (2)
Screenshot 2024-01-30 at 11 38 54 AM (2)

@hi-its-hailey
Copy link
Collaborator

hi-its-hailey commented Jan 30, 2024

Screenshot 2024-01-30 at 11 37 49 AM (2)
@zarahzachz I love this. Could you add "Don't" to the start of every item in the don't column (to avoid it reading like "do this" even with the clear heading)
I guess same with the "Use" column. Start every sentence with "Use" or "don't use" (like the dos and don'ts section)

@hi-its-hailey
Copy link
Collaborator

Screenshot 2024-01-30 at 11 47 36 AM (2)
This is my fault, change the copy to "Each component is tested" instead of "is test"

@hi-its-hailey
Copy link
Collaborator

@zarahzachz Oh, one more thing. The links under "Have ideas" are unchanged. They should be as listed in this image:
Screenshot 2024-01-30 at 12 08 05 PM

@zarahzachz
Copy link
Collaborator Author

@hi-its-hailey for the updated table of contents links, where do those links go? Specifically the "Ask a question" and "Start a contribution"

@zarahzachz
Copy link
Collaborator Author

@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)

@hi-its-hailey
Copy link
Collaborator

hi-its-hailey commented Feb 1, 2024

@zarahzachz

@hi-its-hailey for the updated table of contents links, where do those links go? Specifically the "Ask a question" and "Start a contribution"

Ask a question: Slack
Start a contribution: Github
Contact the team (same as before)
Edit this Page (same as before)

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
Copy link
Collaborator Author

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).
Copy link
Collaborator Author

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">
Copy link
Collaborator Author

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']}>
Copy link
Collaborator Author

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';
Copy link
Collaborator Author

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} />
Copy link
Collaborator Author

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.'
Copy link
Collaborator Author

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}>
Copy link
Collaborator Author

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=""
Copy link
Collaborator Author

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;
Copy link
Collaborator Author

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

@zarahzachz zarahzachz changed the title [WNMGDS-2640] Info architecture update [DON'T MERGE] [WNMGDS-2640] Info architecture update Feb 7, 2024
@zarahzachz zarahzachz changed the title [DON'T MERGE] [WNMGDS-2640] Info architecture update [DON'T MERGE] [WNMGDS-2640 WNMGDS-2160] Info architecture update Feb 7, 2024
<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') }}
Copy link
Collaborator Author

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: {
Copy link
Collaborator

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.

Copy link
Collaborator

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

@zarahzachz
Copy link
Collaborator Author

This work is still in draft mode. The IA needs to be tested with users further and validated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants