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

Fix focus indicators in shields #1894

Closed
tildelowengrimm opened this issue Oct 29, 2018 · 13 comments · Fixed by brave/brave-core#1797
Closed

Fix focus indicators in shields #1894

tildelowengrimm opened this issue Oct 29, 2018 · 13 comments · Fixed by brave/brave-core#1797
Assignees
Labels
bug feature/shields/panel Front-end design and functionality of the Shields panel. feature/shields The overall Shields feature in Brave. priority/P4 Planned work. We expect to get to it "soon". QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/exclude

Comments

@tildelowengrimm
Copy link
Contributor

tildelowengrimm commented Oct 29, 2018

Description

All interactive UI elements in the Shields panel should have keyboard support and the appropriate focus treatment.

Designs

shields focus states

For more CSS and plain text, check Abstract link: https://share.goabstract.com/68d70c4d-8c77-4588-9ee5-ce6418505c79

@tildelowengrimm tildelowengrimm added bug feature/shields The overall Shields feature in Brave. design A design change, especially one which needs input from the design team labels Oct 29, 2018
@tildelowengrimm tildelowengrimm added this to the 1.x Backlog milestone Oct 29, 2018
@tildelowengrimm tildelowengrimm added the priority/P5 Not scheduled. Don't anticipate work on this any time soon. label Oct 31, 2018
@srirambv
Copy link
Contributor

This is on Windows
image

@tildelowengrimm tildelowengrimm added the feature/shields/panel Front-end design and functionality of the Shields panel. label Nov 2, 2018
@tildelowengrimm tildelowengrimm added the design/needs-mock-up needs-mockup A feature which needs design mockup to be implemented. label Nov 6, 2018
@tildelowengrimm
Copy link
Contributor Author

Karen is going to come up with a mock showing the specific outlines for each element.

@rossmoody
Copy link
Contributor

This is something I can adjust in Brave-ui fairly easily. I think it’s a result of us padding the icons instead of margin.

@rossmoody rossmoody self-assigned this Nov 7, 2018
@karenkliu
Copy link

@rossmoody the entire row should get the focus area because it is interactive, not only the icon.

@rossmoody
Copy link
Contributor

Ah, gotcha. Yeah this prob requires further component behavior definition then. I suppose for accessibility the ideal would be to be able to tab from toggle to dropdown in each section.

@rossmoody rossmoody removed their assignment Nov 7, 2018
@karenkliu karenkliu changed the title The focus indicator in shields is kinda weird. Fix focus indicators in shields Nov 16, 2018
@karenkliu
Copy link

@tildelowengrimm tildelowengrimm added priority/P3 The next thing for us to work on. It'll ride the trains. priority/P4 Planned work. We expect to get to it "soon". and removed design A design change, especially one which needs input from the design team design/needs-mock-up needs-mockup A feature which needs design mockup to be implemented. priority/P5 Not scheduled. Don't anticipate work on this any time soon. priority/P3 The next thing for us to work on. It'll ride the trains. labels Nov 28, 2018
@karenkliu
Copy link

Closing this issue because comprehensive design specs and other issues are covered by #2565

@bbondy bbondy modified the milestones: 1.x Backlog, Dupe / Invalid / Not actionable Dec 20, 2018
@karenkliu
Copy link

Reopening this issue because #2565 has been split into smaller issues.

@karenkliu karenkliu reopened this Jan 24, 2019
@cezaraugusto cezaraugusto self-assigned this Jan 29, 2019
@kjozwiak kjozwiak removed this from the Dupe / Invalid / Not actionable milestone Feb 6, 2019
@kjozwiak
Copy link
Member

kjozwiak commented Feb 6, 2019

Reopening this issue because #2565 has been split into smaller issues.

Removing the Dupe / Invalid / Not actionable milestone as per the above.

@karenkliu
Copy link

Hi @cezaraugusto, thanks for your updates on the focus treatment. Here is my design feedback:

focus states design qa

@karenkliu
Copy link

Hi @cezaraugusto, thanks for your update! After hearing your feedback, I think the best workaround is to have the focus indicator on part of the row and not overlapping the toggle or dropdown (I've updated the spec in the OP as well):

shields focus states

Everything else for the focus looks great.

@karenkliu
Copy link

Hi @cezaraugusto, your latest build for these focus states looks good to go from the design side. 👍

@btlechowski
Copy link

btlechowski commented Apr 25, 2019

Verification passed on

Brave 0.64.60 Chromium: 74.0.3729.91 (Official Build) beta (64-bit)
Revision 03844ed83e02b8add3f4b9cb859a7108d55b2e4d-refs/branch-heads/3729@{#860}
OS Windows 10 OS Build 17134.523

Used test plan from brave/brave-core#1797
Checked indicators on main shields page and subpages.
Logged #4225

Verification passed on

Brave 0.64.72 Chromium: 74.0.3729.131 (Official Build) beta(64-bit)
Revision 518a41c1fa7ce1c8bb5e22346e82e42b4d76a96f-refs/branch-heads/3729@{#954}
OS Linux

Verified passed with

Brave 0.64.72 Chromium: 74.0.3729.131 (Official Build) beta(64-bit)
Revision 518a41c1fa7ce1c8bb5e22346e82e42b4d76a96f-refs/branch-heads/3729@{#954}
OS Mac OS X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feature/shields/panel Front-end design and functionality of the Shields panel. feature/shields The overall Shields feature in Brave. priority/P4 Planned work. We expect to get to it "soon". QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants