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

docs: samples #1350

Closed
wants to merge 64 commits into from
Closed

docs: samples #1350

wants to merge 64 commits into from

Conversation

bennypowers
Copy link
Member

@bennypowers bennypowers commented Nov 16, 2023

What I did

  1. add {% sample %} paired shortcode
  2. use it for:
    • card
    • timestamp
  3. tinkered with codeblock's css so that it works nice in grids. did i break stuff? you tell me 😆

Copy link

changeset-bot bot commented Nov 16, 2023

⚠️ No Changeset found

Latest commit: aab12da

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

netlify bot commented Nov 16, 2023

Deploy Preview for red-hat-design-system ready!

Name Link
🔨 Latest commit aab12da
🔍 Latest deploy log https://app.netlify.com/sites/red-hat-design-system/deploys/65cb48f49547610008760c6c
😎 Deploy Preview https://deploy-preview-1350--red-hat-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bennypowers bennypowers changed the title Card/docs/sample docs: samples Nov 16, 2023
Copy link
Contributor

github-actions bot commented Nov 16, 2023

Size Change: +83 B (0%)

Total Size: 223 kB

Filename Size Change
./lib/elements/rh-context-picker/rh-context-picker.js 2.49 kB +83 B (+3%)
ℹ️ View Unchanged
Filename Size
./elements/rh-accordion/rh-accordion-header.js 3.52 kB
./elements/rh-accordion/rh-accordion-panel.js 1.47 kB
./elements/rh-accordion/rh-accordion.js 3.52 kB
./elements/rh-alert/rh-alert.js 4.4 kB
./elements/rh-audio-player/rh-audio-player-about.js 1.85 kB
./elements/rh-audio-player/rh-audio-player-scrolling-text-overflow.js 1.51 kB
./elements/rh-audio-player/rh-audio-player-subscribe.js 1.43 kB
./elements/rh-audio-player/rh-audio-player.js 14.2 kB
./elements/rh-audio-player/rh-cue.js 2 kB
./elements/rh-audio-player/rh-transcript.js 2.94 kB
./elements/rh-avatar/random-pattern-controller.js 2.72 kB
./elements/rh-avatar/rh-avatar.js 2.91 kB
./elements/rh-badge/rh-badge.js 1 kB
./elements/rh-blockquote/rh-blockquote.js 1.94 kB
./elements/rh-button/rh-button.js 4.38 kB
./elements/rh-card/rh-card.js 1.94 kB
./elements/rh-code-block/rh-code-block.js 1.25 kB
./elements/rh-cta/rh-cta.js 4.56 kB
./elements/rh-dialog/rh-dialog.js 4.79 kB
./elements/rh-dialog/yt-api.js 614 B
./elements/rh-footer/rh-footer-block.js 765 B
./elements/rh-footer/rh-footer-copyright.js 362 B
./elements/rh-footer/rh-footer-links.js 1.18 kB
./elements/rh-footer/rh-footer-social-link.js 1.35 kB
./elements/rh-footer/rh-footer-universal.js 4.07 kB
./elements/rh-footer/rh-footer.js 5.08 kB
./elements/rh-footer/rh-global-footer.js 250 B
./elements/rh-menu/rh-menu.js 1.25 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-dropdown.js 2.57 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-menu-section.js 1.46 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-menu.js 1.9 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-overlay.js 572 B
./elements/rh-navigation-secondary/rh-navigation-secondary.js 5.39 kB
./elements/rh-navigation-secondary/test/fixtures.js 851 B
./elements/rh-pagination/rh-pagination.js 4.46 kB
./elements/rh-spinner/rh-spinner.js 1.61 kB
./elements/rh-stat/rh-stat.js 2.24 kB
./elements/rh-subnav/rh-subnav.js 2.83 kB
./elements/rh-surface/rh-surface.js 1.02 kB
./elements/rh-table/rh-sort-button.js 1.44 kB
./elements/rh-table/rh-table.js 3.02 kB
./elements/rh-tabs/rh-tab-panel.js 1.16 kB
./elements/rh-tabs/rh-tab.js 2.8 kB
./elements/rh-tabs/rh-tabs.js 4.78 kB
./elements/rh-tag/rh-tag.js 1.96 kB
./elements/rh-tile/rh-tile-group.js 1.76 kB
./elements/rh-tile/rh-tile.js 4.68 kB
./elements/rh-timestamp/rh-timestamp.js 976 B
./elements/rh-tooltip/rh-tooltip.js 2.24 kB
./lib/context/color/consumer.js 1.15 kB
./lib/context/color/controller.js 1.11 kB
./lib/context/color/provider.js 1.99 kB
./lib/context/event.js 598 B
./lib/context/headings/consumer.js 711 B
./lib/context/headings/controller.js 1.14 kB
./lib/context/headings/provider.js 1.23 kB
./lib/DirController.js 569 B
./lib/elements/rh-context-demo/rh-context-demo.js 1.14 kB
./lib/functions.js 175 B
./lib/I18nController.js 1.38 kB
./lib/ScreenSizeController.js 856 B
./rhds.min.js 81.6 kB

compressed-size-action

@bennypowers
Copy link
Member Author

I don't think this needs to be comprehensive. the only blockers on this should be things like:

  • the design needs improvements
  • something is broken

but, like .. "we should also do this for xyz element" can be a follow up pr

@bennypowers bennypowers marked this pull request as ready for review November 23, 2023 13:56
Copy link
Collaborator

@zeroedin zeroedin left a comment

Choose a reason for hiding this comment

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

https://deploy-preview-1350--red-hat-design-system.netlify.app/elements/accordion/style/#theme

Color Palette isn't clickable here in Chrome 120.0.6099.199:

image

Doesn't display at all in Safari 17.2.1 :

image

And is weirdly cropped in Firefox 115.6.0esr (64-bit)

Screenshot 2024-01-08 at 1 47 10 PM

@markcaron
Copy link
Collaborator

General comment non blocker to this PR. Is the intent to replace all "sample" images with real working components? If so should we get those components not in this PR added to the backlog to follow up on. /cc @markcaron

@zeroedin not necessarily all sample images, but at least the ones that make the most sense.

@bennypowers bennypowers requested a review from zeroedin January 10, 2024 19:58
@bennypowers
Copy link
Member Author

the anatomy sample component is at the MVP level. if that's the only blocker, slap a hidden attr on it and let's merge this puppy

@markcaron
Copy link
Collaborator

@bennypowers honestly, I feel like the Sample Component has gone beyond the original intention.

Don't get me wrong, it's looking pretty cool with all the features, but I think it changes the context of the design sections, like the Style tab. We don't want to replace ALL images, since images still have their use in these sections.

For example, if you're wanting to see all the different Sizes of the Audio Player, it looks "fine" (aside from the last one breaking out of the container):

image

But, on Mobile, you lose the visual since ALL of the Audio Player samples become "mobile friendly":

image

We want the image to show ALL the sizes/options, and not have the visual representation / Sample Component react live to the browser or container width.

Note: the little purple tooltips are also wonky on mobile, but that's a separate topic

I feel like the true use of the Sample Component lives on the Code and Overview tabs, where we'd want to show a live example, and not on design sections where we'd want to highlight specific features (that retain their context across devices).

For example, on mobile, the Configuration section is using an image still, and it doesn't lose it's context—you can still see all the config options visually:

image

Hopefully that makes sense.

This isn't a blocker anyway. Maybe we take this out of the Caterpie release, and fast follow with it since it's just docs.

@bennypowers
Copy link
Member Author

bennypowers commented Jan 11, 2024

yup, makes sense to me. TL;DR: I'm happy to save this for chancey, with modifications

  • audio player mini size breaking out of container - should we look at this as something to fix in the player? meaning, that the minimum total width of the mini player should be adjusted down so as to match the original mockups on that page
  • sizes should be images - agreed. I'll revert those ones. Something to think about for the future: @zeroedin is researching container queries right now. I wonder how feasible it would be to develop a live, real-html version of that "sizes" image, scaled down to fit mobile screens. Would that be possible if audio player used @container in place of @media?
  • general wonkiness of live anatomy component - yeah the thing is still in the prototype stage, so there's plenty of spit-and-shine left to do. Perhaps there's room to have at least one live anatomy sample per-page, even if it doesn't completely replace the prepared images?

I spun out #1413 with all the "by-the-way" fixes from this PR

@zeroedin
Copy link
Collaborator

Do we want to hold on uxdot-anatomy-sample until we have time to flesh it out more? I think it would be nice to get the uxdot-code-sample component in without that anatomy holding this PR up.

@bennypowers
Copy link
Member Author

Superceeded mostly by #1465

auto-merge was automatically disabled May 21, 2024 14:19

Pull request was closed

@bennypowers bennypowers deleted the card/docs/sample branch June 19, 2024 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done ☑️
Development

Successfully merging this pull request may close these issues.

4 participants