red-hat-design-system
red-hat-design-system copied to clipboard
fix(audio-player): use rh-icon instead of custom svg for scrubbers
What I did
- Replaced custom svg's with
rh-iconfor scrubbers
Closes #2049
TODO:
- [ ] Docs images as listed in issue
Testing Instructions
Notes to Reviewers
@marionnegp I added the rh-icon with the same size as the play button (24px) as that looked closest to the previous state. If it should be a different size let me know. I'll leave PR in draft and if you want to make the changes to the docs images on this same branch go right ahead.
🦋 Changeset detected
Latest commit: 8c8f0014ce81e0b16841ebea9bfd0f91c74bbb7a
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @rhds/elements | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
Deploy Preview for red-hat-design-system ready!
| Name | Link |
|---|---|
| Latest commit | 8c8f0014ce81e0b16841ebea9bfd0f91c74bbb7a |
| Latest deploy log | https://app.netlify.com/sites/red-hat-design-system/deploys/67a873610b7d170008288d21 |
| Deploy Preview | https://deploy-preview-2095--red-hat-design-system.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Size Change: -450 B (-0.22%)
Total Size: 206 kB
| Filename | Size | Change |
|---|---|---|
./elements.js |
473 B | -3 B (-0.63%) |
./elements/rh-audio-player/rh-audio-player.js |
12.6 kB | -447 B (-3.41%) |
ℹ️ View Unchanged
| Filename | Size |
|---|---|
./elements/rh-accordion/context.js |
162 B |
./elements/rh-accordion/rh-accordion-header.js |
2.69 kB |
./elements/rh-accordion/rh-accordion-panel.js |
1.35 kB |
./elements/rh-accordion/rh-accordion.js |
3.21 kB |
./elements/rh-alert/rh-alert.js |
4.26 kB |
./elements/rh-audio-player/rh-audio-player-about.js |
1.8 kB |
./elements/rh-audio-player/rh-audio-player-rate-stepper.js |
1.85 kB |
./elements/rh-audio-player/rh-audio-player-scrolling-text-overflow.js |
1.53 kB |
./elements/rh-audio-player/rh-audio-player-subscribe.js |
1.38 kB |
./elements/rh-audio-player/rh-cue.js |
1.95 kB |
./elements/rh-audio-player/rh-transcript.js |
2.7 kB |
./elements/rh-avatar/random-pattern-controller.js |
2.72 kB |
./elements/rh-avatar/rh-avatar.js |
2.86 kB |
./elements/rh-back-to-top/rh-back-to-top.js |
2.1 kB |
./elements/rh-badge/rh-badge.js |
1.55 kB |
./elements/rh-blockquote/rh-blockquote.js |
1.36 kB |
./elements/rh-breadcrumb/rh-breadcrumb.js |
1.5 kB |
./elements/rh-button/rh-button.js |
4.24 kB |
./elements/rh-card/rh-card.js |
3.58 kB |
./elements/rh-code-block/prism.css.js |
667 B |
./elements/rh-code-block/prism.js |
572 B |
./elements/rh-code-block/rh-code-block.js |
7.34 kB |
./elements/rh-cta/rh-cta.js |
4.04 kB |
./elements/rh-dialog/rh-dialog.js |
4.78 kB |
./elements/rh-dialog/yt-api.js |
617 B |
./elements/rh-footer/rh-footer-block.js |
714 B |
./elements/rh-footer/rh-footer-copyright.js |
362 B |
./elements/rh-footer/rh-footer-links.js |
1.17 kB |
./elements/rh-footer/rh-footer-social-link.js |
964 B |
./elements/rh-footer/rh-footer-universal.js |
3.99 kB |
./elements/rh-footer/rh-footer.js |
4.96 kB |
./elements/rh-health-index/rh-health-index.js |
2.35 kB |
./elements/rh-icon/rh-icon.js |
2.47 kB |
./elements/rh-icon/ssr.js |
181 B |
./elements/rh-menu/rh-menu.js |
1.29 kB |
./elements/rh-navigation-secondary/rh-navigation-secondary-dropdown.js |
2.47 kB |
./elements/rh-navigation-secondary/rh-navigation-secondary-menu-section.js |
1.3 kB |
./elements/rh-navigation-secondary/rh-navigation-secondary-menu.js |
1.75 kB |
./elements/rh-navigation-secondary/rh-navigation-secondary-overlay.js |
571 B |
./elements/rh-navigation-secondary/rh-navigation-secondary.js |
5.26 kB |
./elements/rh-navigation-secondary/test/fixtures.js |
769 B |
./elements/rh-pagination/rh-pagination.js |
5.4 kB |
./elements/rh-site-status/rh-site-status.js |
2.5 kB |
./elements/rh-skip-link/rh-skip-link.js |
1.19 kB |
./elements/rh-spinner/rh-spinner.js |
1.38 kB |
./elements/rh-stat/rh-stat.js |
2.13 kB |
./elements/rh-subnav/rh-subnav.js |
2.73 kB |
./elements/rh-surface/rh-surface.js |
1.14 kB |
./elements/rh-surface/test/elements.js |
423 B |
./elements/rh-switch/rh-switch.js |
2.93 kB |
./elements/rh-table/rh-sort-button.js |
1.49 kB |
./elements/rh-table/rh-table.js |
3.48 kB |
./elements/rh-tabs/context.js |
160 B |
./elements/rh-tabs/rh-tab-panel.js |
1.04 kB |
./elements/rh-tabs/rh-tab.js |
3.02 kB |
./elements/rh-tabs/rh-tabs.js |
3.77 kB |
./elements/rh-tag/rh-tag.js |
2.79 kB |
./elements/rh-tile/rh-tile-group.js |
1.81 kB |
./elements/rh-tile/rh-tile.js |
5.09 kB |
./elements/rh-timestamp/rh-timestamp.js |
983 B |
./elements/rh-tooltip/rh-tooltip.js |
2.73 kB |
./elements/rh-video-embed/rh-video-embed.js |
4.59 kB |
./lib/context/color/consumer.js |
1.41 kB |
./lib/context/color/controller.js |
947 B |
./lib/context/color/provider.js |
2.18 kB |
./lib/context/event.js |
593 B |
./lib/context/headings/consumer.js |
722 B |
./lib/context/headings/controller.js |
1.12 kB |
./lib/context/headings/provider.js |
1.24 kB |
./lib/DirController.js |
565 B |
./lib/elements/rh-context-demo/rh-context-demo.js |
1.28 kB |
./lib/elements/rh-context-picker/rh-context-picker.js |
2.24 kB |
./lib/environment.js |
194 B |
./lib/functions.js |
175 B |
./lib/I18nController.js |
1.38 kB |
./lib/ScreenSizeController.js |
849 B |
./lib/ssr-controller.js |
251 B |
./react/rh-accordion/rh-accordion-header.js |
199 B |
./react/rh-accordion/rh-accordion-panel.js |
185 B |
./react/rh-accordion/rh-accordion.js |
215 B |
./react/rh-alert/rh-alert.js |
184 B |
./react/rh-audio-player/rh-audio-player-about.js |
191 B |
./react/rh-audio-player/rh-audio-player-rate-stepper.js |
213 B |
./react/rh-audio-player/rh-audio-player-scrolling-text-overflow.js |
214 B |
./react/rh-audio-player/rh-audio-player-subscribe.js |
196 B |
./react/rh-audio-player/rh-audio-player.js |
183 B |
./react/rh-audio-player/rh-cue.js |
195 B |
./react/rh-audio-player/rh-transcript.js |
207 B |
./react/rh-avatar/rh-avatar.js |
173 B |
./react/rh-back-to-top/rh-back-to-top.js |
183 B |
./react/rh-badge/rh-badge.js |
174 B |
./react/rh-blockquote/rh-blockquote.js |
179 B |
./react/rh-breadcrumb/rh-breadcrumb.js |
179 B |
./react/rh-button/rh-button.js |
174 B |
./react/rh-card/rh-card.js |
172 B |
./react/rh-code-block/rh-code-block.js |
181 B |
./react/rh-cta/rh-cta.js |
170 B |
./react/rh-dialog/rh-dialog.js |
203 B |
./react/rh-footer/rh-footer-block.js |
184 B |
./react/rh-footer/rh-footer-copyright.js |
187 B |
./react/rh-footer/rh-footer-links.js |
185 B |
./react/rh-footer/rh-footer-social-link.js |
193 B |
./react/rh-footer/rh-footer-universal.js |
188 B |
./react/rh-footer/rh-footer.js |
174 B |
./react/rh-health-index/rh-health-index.js |
184 B |
./react/rh-icon/rh-icon.js |
202 B |
./react/rh-menu/rh-menu.js |
173 B |
./react/rh-navigation-secondary/rh-navigation-secondary-dropdown.js |
217 B |
./react/rh-navigation-secondary/rh-navigation-secondary-menu-section.js |
205 B |
./react/rh-navigation-secondary/rh-navigation-secondary-menu.js |
199 B |
./react/rh-navigation-secondary/rh-navigation-secondary-overlay.js |
201 B |
./react/rh-navigation-secondary/rh-navigation-secondary.js |
213 B |
./react/rh-pagination/rh-pagination.js |
178 B |
./react/rh-site-status/rh-site-status.js |
181 B |
./react/rh-skip-link/rh-skip-link.js |
181 B |
./react/rh-spinner/rh-spinner.js |
175 B |
./react/rh-stat/rh-stat.js |
171 B |
./react/rh-subnav/rh-subnav.js |
175 B |
./react/rh-surface/rh-surface.js |
175 B |
./react/rh-switch/rh-switch.js |
185 B |
./react/rh-table/rh-sort-button.js |
213 B |
./react/rh-table/rh-table.js |
174 B |
./react/rh-tabs/rh-tab-panel.js |
181 B |
./react/rh-tabs/rh-tab.js |
187 B |
./react/rh-tabs/rh-tabs.js |
174 B |
./react/rh-tag/rh-tag.js |
182 B |
./react/rh-tile/rh-tile-group.js |
183 B |
./react/rh-tile/rh-tile.js |
194 B |
./react/rh-timestamp/rh-timestamp.js |
176 B |
./react/rh-tooltip/rh-tooltip.js |
175 B |
./react/rh-video-embed/rh-video-embed.js |
227 B |
./uxdot/uxdot-best-practice.js |
742 B |
./uxdot/uxdot-copy-button.js |
1.2 kB |
./uxdot/uxdot-copy-permalink.js |
1.1 kB |
./uxdot/uxdot-example.js |
1.17 kB |
./uxdot/uxdot-feedback.js |
727 B |
./uxdot/uxdot-header.js |
1.07 kB |
./uxdot/uxdot-hero.js |
679 B |
./uxdot/uxdot-installation-tabs.js |
675 B |
./uxdot/uxdot-masthead.js |
809 B |
./uxdot/uxdot-pattern-ssr-controller-client.js |
604 B |
./uxdot/uxdot-pattern-ssr-controller-server.js |
1.68 kB |
./uxdot/uxdot-pattern-ssr-controller.js |
213 B |
./uxdot/uxdot-pattern.js |
2.12 kB |
./uxdot/uxdot-repo-status-checklist.js |
1.16 kB |
./uxdot/uxdot-repo-status-list.js |
1.07 kB |
./uxdot/uxdot-repo-status-table.js |
782 B |
./uxdot/uxdot-repo.js |
1.17 kB |
./uxdot/uxdot-search.js |
2.39 kB |
./uxdot/uxdot-sidenav.js |
2.67 kB |
./uxdot/uxdot-spacer-tokens-table.js |
2.45 kB |
./uxdot/uxdot-toc.js |
1.13 kB |
- Speed controls don't seem to be using the right color in dark theme.
This looks like it opens up a bigger issue on audio player. Will need to dig deeper tomorrow to see what is going on, Color context from the outside leaks into audio player and it has an internal surface which doesn't set to match the parent context:
Example of issue:
<rh-context-demo color-palette="darkest"> <!--sets context to dark-->
<rh-audio-player> <!-- dark background is set -->
<!-- shadowroot-->
<rh-surface> <!-- internal context is set to light (note no color palette) -->
Text / Buttons <!-- all think they are on a light background -->
</rh-surface>
</rh-audio-player>
</rh-context-demo>
@bennypowers I can't believe the resolution was as simple as removing the rh-surface from the shadow root. However, I couldn't find a reason for it. Can you double-check me here? I may be overlooking something.
Play button in light theme player should be in a darkest circle with a white play icon:
@marionnegp, I'll look at these changes with the circle not being present on the play icon now. The circle was likely part of the custom SVG previously.
The circle was likely part of the custom SVG previously.
This was not a true statement 😆 , that said I'm now adjusting the variables used directly based on the context.
#play,
#full-play {
border-radius: 50%;
background-color: var(--rh-color-surface-darkest, #151515);
color:
var(--rh-audio-player-icon-background-color,
var(--rh-color-text-primary-on-dark, #ffffff));
}
.dark {
#play,
#full-play {
background-color: var(--rh-color-surface-lightest, #ffffff);
color:
var(--rh-audio-player-icon-background-color,
var(--rh-color-text-primary-on-light, #151515));
}
}
@marionnegp I feel like the play icon may need micro-adjusting to make it feel centered (visually). I think we've talked about this before somewhere. Déjà vu?
Another bug found, the highlight in the text being read in the transcript also appears to be off.
Does not appear to be new / introduced in this PR either, also found on ux.redhat.com
@marionnegp this feature set is not outlined in the Figma file. Guidance here on what this should be is appreciated.
I think we've talked about this before somewhere. Déjà vu?
It was in rh-video-embed. it looks like the difference is we need to use an rh-button variant="play" which will get us the adjusted variant we are looking for. Looking to see if we can reuse that here.
Another bug found, the highlight in the text being read in the transcript also appears to be off.
The docs show images of the highlight for light theme, and that blue looks most like --rh-color-blue-20. @coreyvickery, did you specify a highlight color for light and dark themes?
Looking at Nikki's original audio player PR and the transcript demo, it might have been using the default browser highlight. Maybe this was a bug introduced when we made changes to the theming?
Maybe this was a bug introduced when we made changes to the theming?
I believe that is the case, yes.
Noting the auto scroll download toolbar is now missing from demos on ux.redhat.com
Should look like:
Now looks like:
Given changes in theming between the opening of this PR and its becoming stale, I recommend closing this PR in favor of opening a newly cut branch of the current state main to deal with this issue and reimplement the changes instead of dealing with the conflicts that are bound to happen here. Leaving it open till a new PR is cut that references this one.
Close in favor of: https://github.com/RedHat-UX/red-hat-design-system/pull/2469