-
Notifications
You must be signed in to change notification settings - Fork 18
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
docs(elements): update images and summaries #1007
Conversation
|
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Size Change: +116 B (0%) Total Size: 188 kB
ℹ️ View Unchanged
|
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.
Looking good so far with a couple changes on the DP:
- Elements should be sorted by and alias not tagName
- Elements should be displaying alias not tagName ("Cta" should be "Call to Action", "Stat" should be "Statistic", etc.)
- Card's summary does not match the Doc
- Pagination has no summary displaying
- Foundations section at the bottom should be replaced with Patterns
@nikkimk Thanks for the review. I think I've addressed all the issues you identified. Can you take another looksy, pleaseee? <3 |
@eyevana Some very small edits:
To request a new element or if updates need to be made to an existing element, create a GitHub issue. The link above should open a new tab or window. |
@eyevana Hey, I do not see the |
@coreyvickery yep, I'm working on that last piece now. I'll ping you here when it's ready for another round of reviews. |
@coreyvickery I added Breadcrumb and Footnote to /elements as coming soon. Do you also want me to include them in the side nav? For example, "Breadcrumb (coming soon)" with no link? |
@eyevana No thanks. |
@bennypowers @nikkimk can you take a look at how I handled the "coming soon" elements and let me know what you think? I stubbed them under the |
@eyevana Sorry, I forgot to mention that we are removing |
@coreyvickery exclude |
@eyevana Yes, kill it. Or create a new issue to kill it website-wide. |
@coreyvickery ready for you 👁️ ❤️ |
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.
lgtm
@nikkimk thank you! |
@coreyvickery I'm reverting the change where I remove subnav from the docs. We were chatting about removing subnav during office hours, and the team agrees we should keep it since it's still in use. Steven already broke it out into an issue #1029. |
This reverts commit 3f35bcf.
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.
pls move images back to elements/*/docs/screenshot.png, as they can be used on other listings, not only on ux-dot
@eyevana looks like there are still some conflicts to resolve |
Done! :) |
Actually need to make one more change |
Closes #1008
What I did
/elements
landing page according to the design specexample
shortcode to allow for absolute paths in its image srcTesting Instructions
Notes to Reviewers