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

GIVCAMP-282 | React Player preview (light) #216

Merged
merged 14 commits into from
Apr 5, 2024

Conversation

yvonnetangsu
Copy link
Member

@yvonnetangsu yvonnetangsu commented Jan 30, 2024

READY FOR REVIEW

Summary

  • Add storyblok option to toggle embed media preview (light) mode

Review By (Date)

  • Retro

Criticality

  • 2

Review Tasks

Setup tasks and/or behavior to test

  1. Go to https://deploy-preview-216--giving-campaign.netlify.app/stories/education-for-a-life-of-purpose
  2. Look at videos in preview mode and compared to Kerri's mock (just look at the preview icon)
    Screenshot 2024-01-29 at 1 57 04 PM
  3. Inspect the preview div and see that there's an aria-label with something like "Play video......"
  4. Click on preview and see video starts playing

Associated Issues and/or People

Copy link

netlify bot commented Jan 30, 2024

Deploy Preview for giving-campaign ready!

Name Link
🔨 Latest commit 99be874
🔍 Latest deploy log https://app.netlify.com/sites/giving-campaign/deploys/660f31062bfae200089058d8
😎 Deploy Preview https://deploy-preview-216--giving-campaign.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.

yvonnetangsu and others added 10 commits January 30, 2024 16:16
* dev: (34 commits)
  GIVCAMP-292 GIVCAMP-305 | Horizontal Initiative card and image aspect ratio options (#256)
  1.3.3
  Update robots to use different URL for sitemap path (#254)
  1.3.2
  GIVCAMP-307 | Stacked story hero variant (#251)
  1.3.1
  GIVCAMP-297 GIVCAMP-312 | Section background options + add light overlay options (#248)
  Bump follow-redirects from 1.15.4 to 1.15.6 (#249)
  1.3.0
  GIVCAMP-289: Inline External Script Loading. (#246)
  GIVCAMP-88 | data card (#245)
  1.2.2
  GIVCAMP-294 | Basic Hero overlay options and image optimization (#243)
  HSTS set max-age to 31536000 (#242)
  1.2.1
  Icon animation (#240)
  GIVCAMP-304 | Add CTA region to Section and Homepage Theme/Story Section (#239)
  1.2.0
  GIVCAMP-293 | Moment poster (#236)
  NoJira: Remove Editor Token (#237)
  ...
@yvonnetangsu yvonnetangsu changed the title POST LAUNCH - GIVCMP-282 | React Player preview (light) =GIVCAMP-282 | React Player preview (light) Apr 2, 2024
@yvonnetangsu yvonnetangsu changed the title =GIVCAMP-282 | React Player preview (light) GIVCAMP-282 | React Player preview (light) Apr 2, 2024
@yvonnetangsu yvonnetangsu marked this pull request as ready for review April 2, 2024 23:41
Comment on lines +78 to +79
// This previewAriaLabel prop is not documented but it is in the React Player source code
previewAriaLabel={isPreview ? previewAriaLabel : undefined}
Copy link
Member Author

Choose a reason for hiding this comment

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

SODA said this makes it ok accessible (it renders a clickable/focusable div). When I have time I'll open a PR to React Player to add a role="button" (SODA suggestion to make it fully accessible).

Copy link
Member

Choose a reason for hiding this comment

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

You can always just log a request or file a bug. It is nice of you to submit the PR but sometimes you can just let others know if you don't have the time.

@@ -40,7 +40,7 @@
"framer-motion": "^10.16.4",
"next": "13.4.9",
"react-loading-skeleton": "^3.3.1",
"react-player": "^2.14.1",
"react-player": "^2.15.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

Update react player to latest which included this new previewAriaLabel prop

className="lg:col-span-6 bg-black-70"
aspectRatio="16x9"
previewAriaLabel={previewAriaLabel}
Copy link
Member

Choose a reason for hiding this comment

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

Yay reading through the code...

When things are not in the doxn it scares me a bit for if the maintainers are going to continue to maintain the prop. Most of the time it is just because they haven't gotten to the documentation yet and I don't see this as a critical thing but I thought I would share my $0.02 anyhow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it looks like they recently switched maintainer. I looked at the merged pull request, and looks like this prop was added fairly recently:
cookpete/react-player#1705

I'll give them benefit of the doubt this time 😆

@sherakama
Copy link
Member

sherakama commented Apr 5, 2024

Screenshot 2024-04-05 at 09 39 10

This is looking and working just fine but I noticed that the double quotes are there. You may have to escape or encode the text going in.

Looks like unicode FTW here.

Copy link
Member

@sherakama sherakama left a comment

Choose a reason for hiding this comment

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

GTG

@yvonnetangsu
Copy link
Member Author

Screenshot 2024-04-05 at 09 39 10

This is looking and working just fine but I noticed that the double quotes are there. You may have to escape or encode the text going in.

Looks like unicode FTW here.

Thanks for checking @sherakama ! 😄

@yvonnetangsu yvonnetangsu merged commit 183daa9 into dev Apr 5, 2024
5 checks passed
@yvonnetangsu yvonnetangsu deleted the feature/GIVCAMP-282_react-player-light branch April 5, 2024 17:17
@yvonnetangsu yvonnetangsu mentioned this pull request Apr 5, 2024
yvonnetangsu added a commit that referenced this pull request Apr 6, 2024
yvonnetangsu added a commit that referenced this pull request Apr 9, 2024
* dev:
  1.4.0
  GIVCAMP-313 | Vertical poster component with parallax (#253)
  1.3.5
  GIVCAMP-282 | React Player preview (light) (#216)
  1.3.4
  Update sitemap.ts (#257)

# Conflicts:
#	components/Parallax/Parallax.tsx
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.

2 participants