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

Removing Border for select / input and options, Issue #440 #447

Merged
merged 4 commits into from
Oct 12, 2020

Conversation

ccsCoder
Copy link
Contributor

Related issue

Closes #440

Context / Background

The border didn't look good when dark theme was used. It resulted in an Inconsistent visual look and feel across the application.

What change is being introduced by this PR?

  • Inspected the elements in question, identified the styles and changed them
  • Changed #preferences input/select/option and set border: none.
  • Direct consequence: the borders are removed as intended, indirect consequence is that we now have a more uniform look and feel.

How will this be tested?

  • We will verify that the borders are removed indeed and check the same across themes.

@thamara
Copy link
Owner

thamara commented Oct 12, 2020

@ccsCoder Can you please provide screenshots?

@codecov
Copy link

codecov bot commented Oct 12, 2020

Codecov Report

Merging #447 into main will decrease coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #447      +/-   ##
==========================================
- Coverage   64.07%   63.95%   -0.12%     
==========================================
  Files          28       28              
  Lines        2536     2536              
  Branches      389      389              
==========================================
- Hits         1625     1622       -3     
- Misses        805      806       +1     
- Partials      106      108       +2     
Impacted Files Coverage Δ
js/update-manager.js 61.53% <0.00%> (-7.70%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fc311b...badf537. Read the comment docs.

@thamara
Copy link
Owner

thamara commented Oct 12, 2020

Right after selecting the item, it still shows an outline. Can you please make it outline: 0?

@ccsCoder
Copy link
Contributor Author

Screenshots:
Dark
Screen Shot 2020-10-12 at 11 01 20 AM

Cadet Star
Screen Shot 2020-10-12 at 11 01 41 AM

Light
Screen Shot 2020-10-12 at 11 00 50 AM

@ccsCoder
Copy link
Contributor Author

ccsCoder commented Oct 12, 2020

Right after selecting the item, it still shows an outline. Can you please make it outline: 0?

I think that is not a good idea. In-fact I was going to raise an issue about it.
Focus rings / Outlines are good for Accessibility. It tells Keyboard users, and screen readers which item is under focus.

We should make the other elements Focusable and interactable through keyboard also as per WAI-ARIA guidelines.

I can take this up if you're okay with this. 👍

Of-Course we can change the colour / style of the outline / focus ring.

@thamara
Copy link
Owner

thamara commented Oct 12, 2020

Hmm, that's interesting, I haven't paid as much attention to it as I probably should have (given my background doesn't include anything UX-related it would be hard anyway hahaha).

Maybe we could just change the outline color for the focused item (I suppose this is something we can do), just to match the themes better.

We should make the other elements Focusable and interactable through keyboard also as per WAI-ARIA guidelines.
I can take this up if you're okay with this. 👍

I guess I have some reading to do regarding WAI-ARIA... But it does sound great! Feel free to work on those, and if you have any other suggestions regarding accessibility, keep them coming! :)

@ccsCoder
Copy link
Contributor Author

Hmm, that's interesting, I haven't paid as much attention to it as I probably should have (given my background doesn't include anything UX-related it would be hard anyway hahaha).

Maybe we could just change the outline color for the focused item (I suppose this is something we can do), just to match the themes better.

We should make the other elements Focusable and interactable through keyboard also as per WAI-ARIA guidelines.
I can take this up if you're okay with this. 👍

I guess I have some reading to do regarding WAI-ARIA... But it does sound great! Feel free to work on those, and if you have any other suggestions regarding accessibility, keep them coming! :)

I wouldn't worry about it.
Sad truth is that most websites / applications are not developed with a11y in mind. I will change the outline colours for the time being.

@thamara
Copy link
Owner

thamara commented Oct 12, 2020

Thank you a lot!

Sad truth is that most websites / applications are not developed with a11y in mind.

I'll be adding a goal for the project to become more accessible, without a question this is something we would like to support, even we don't really know how to (yet).

@ccsCoder
Copy link
Contributor Author

Thank you a lot!

Sad truth is that most websites / applications are not developed with a11y in mind.

I'll be adding a goal for the project to become more accessible, without a question this is something we would like to support, even we don't really know how to (yet).

Luckily, I work a lot on a11y. I'd love to put those skills to some use and help you out.

@thamara
Copy link
Owner

thamara commented Oct 12, 2020

Luckily, I work a lot on a11y. I'd love to put those skills to some use and help you out.

That would be absolutely awesome!

@ccsCoder
Copy link
Contributor Author

ccsCoder commented Oct 12, 2020

Screenshots of Outline / focus ring.

Dark:
Screen Shot 2020-10-12 at 11 36 33 AM

Light:
Screen Shot 2020-10-12 at 11 35 26 AM

Cadent Star:
Screen Shot 2020-10-12 at 11 36 00 AM

We can iterate over the L&F of the focus ring.

@ccsCoder
Copy link
Contributor Author

@thamara please check now.

Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Apart from the color change, everything looks great!
Thank you a lot!

css/styles.css Outdated Show resolved Hide resolved
ccsCoder and others added 2 commits October 12, 2020 11:56
Co-authored-by: Thamara Andrade <tkcandrade@gmail.com>
@thamara
Copy link
Owner

thamara commented Oct 12, 2020

\changelog-update
Message: Accessibility: [#447] Including focus ring/outline for inputs in the App
User: cssCoder

@thamara thamara merged commit fa91728 into thamara:main Oct 12, 2020
@ccsCoder ccsCoder deleted the issue-440 branch October 12, 2020 06:34
thamara added a commit to jcombs0929/time-to-leave that referenced this pull request Oct 17, 2020
…amara#447)

* Removing Border for select / input and options

* Added Theme appropriate outlines to Controls. removed outline : none .

* Update css/styles.css

Co-authored-by: Thamara Andrade <tkcandrade@gmail.com>

* Style violation fixed: changed to 2 spaces

Co-authored-by: Thamara Andrade <tkcandrade@gmail.com>
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.

Remove border of input boxed in preferences window
2 participants