Skip to content

Layout changes accessibility #111

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

Merged
merged 32 commits into from
Mar 18, 2019

Conversation

ivanji
Copy link

@ivanji ivanji commented Mar 14, 2019

Is your Pull Request request related to another issue in this repository ?
#50

Describe what the PR does
Continuing on work done by @pietrop on #70

State whether the PR is ready for review or whether it needs extra work
Ready for review

Additional context
editor-layout-changes

Pietro Passarelli - News Labs and others added 30 commits January 3, 2019 19:43
to accomodate for two new fields
leaving rest for another branch/PR tbc
This reverts commit 2c6a9dd.
removed volume component and added titles tags to player controls btns
eg linting, css etc.. from James' PR feedback
+ Range bar re-styled
@pietrop
Copy link
Contributor

pietrop commented Mar 14, 2019

Thanks for this @ivanji , we are having a closer look with @jamesdools tomorrow.

I reckon we could hide the Picture in picture icon in mobile view as it doesn't seem be supported in iOS, or android, and that could make more space for other icons.

@ivanji
Copy link
Author

ivanji commented Mar 14, 2019

Sure, thanks. BTW, I'm going to push a couple more changes as I've been testing further and found some minor issues. I'll get rid of PiP in mobile view and bring back the sound control button, it's an accessibility issue not having it and there's no reason why it shouldn't be present.

@pietrop
Copy link
Contributor

pietrop commented Mar 14, 2019

ok, cool, thanks @ivanji !

re the sound toggle - ok that's good.

A few other things I've noticed

Progress bar
In firefox it's a red dot while in chrome it's white?

Current Timecode - colour
Colour of current time, looks a bit faded #E2A9A2 instead of #a0372d - was wondering what's the reason behind that?

Position of settings icon, and keyboard shortcuts icon
Position of settings icon, and keyboard shortcuts icon in mobile view seems to go a bit off to the top of the page, rather then staying with the component, see below

Screen Shot 2019-03-14 at 22 37 06

ToolTip
Also "how does this work" tooltip, doesn't seem to work?

Screen Shot 2019-03-14 at 22 43 34

+ Added Toggle sound
@ivanji
Copy link
Author

ivanji commented Mar 15, 2019

I've fixed these issues and some minor ones I also noticed. I added Toggle Sound on/off button (instead of volume control as realistically that's what it is).

Colour of current time, looks a bit faded #E2A9A2 instead of #a0372d - was wondering what's the reason behind that?

Accessibility! previous red fails contrast checks. FYI, I didn't remove it, just added a new colour but like anything, it's to the devs what style they ultimately decide on, but hopefully always considering contrast. As a default option we might as well comply with best guidelines :)

@jamesdools jamesdools merged commit 89b5d16 into bbc:master Mar 18, 2019
@ivanji ivanji deleted the layout-changes-accessibility branch March 19, 2019 21:47
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.

3 participants