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

704 add instructor approved i note to playlist screen for each video #829

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from

Conversation

mumbler6
Copy link
Contributor

Fixes #704

Adds I-note button to each video in the watch playlist menu, which links to i-notes created for this video.

image

First video's i-note button links here:

image

I was having trouble adjusting the styling to something nicer, so if anyone wants to change things that would be great.

@mumbler6 mumbler6 linked an issue Jul 27, 2024 that may be closed by this pull request
@angrave
Copy link
Collaborator

angrave commented Jul 29, 2024

In the first screenshot of the playlist media cards there's insufficient contrast for the buttons ; The new I-Note button text is too dark i.e. brightness is too close to the black background to be legible.

@mumbler6
Copy link
Contributor Author

In the first screenshot of the playlist media cards there's insufficient contrast for the buttons ; The new I-Note button text is too dark i.e. brightness is too close to the black background to be legible.

What do you think about this? The green is when you are hovering over it.

image

@angrave
Copy link
Collaborator

angrave commented Jul 30, 2024

Green hover. Unfortunately this does not meet accessibility requirements: Contrast must always be sufficient.

Also not everyone uses a mouse (e.g. instead uses tab to move focus) - so hover wouldnt work for that use case.
Also the green still looks very dark against the background - but we'd need to use a color checking tool to confirm if it is too dark. (But to my eyes I think it is)

@mumbler6
Copy link
Contributor Author

mumbler6 commented Jul 30, 2024

I'm using the same teal green color that is being used currently when hovering over a video:

image

I can make it this lighter green maybe:

image

Thanks for pointing out the tab issue I'll try to add something to incorporate that. I also want to add that I-note button is now white when not being hovered over / focused on, so the contrast there is sufficient right?

@angrave
Copy link
Collaborator

angrave commented Jul 30, 2024

White when not hover; yes - that will be great.
Re Tab: It's standard to NOT set tab index. More info here
https://www.w3.org/WAI/WCAG21/Understanding/focus-order.html

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.

Add Instructor approved I-Note to playlist screen for each video
2 participants