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

minor accessibility fixes #3297 #3463

Closed
wants to merge 5 commits into from

Conversation

derekriemer
Copy link
Contributor

@derekriemer derekriemer commented Jul 30, 2016

Purpose

This fixes a few accessibility issues, mainly the state of certain controls.

Testing

Testing in mozilla firefox, Google Chrome, and Internet explorer with the NVDA screen reader have been done. I'm not sure if this project has integration tests.

Screenshots

This is a GUI change, but the GUI should not visually have changed.

derekriemer added 2 commits June 17, 2016 08:27
* add collapsed labels to various things.
* Make the dropdowns act as menus
* Provide tab key navigation to the tabs in the advanced settings.
* Add semicolon to the style in a couple of places.
@derekriemer
Copy link
Contributor Author

Hi, Do you guys prefer rebasing or merging onto master?
I can merge master into this to fix the issues, but would a rebase be prefered?

@calmh
Copy link
Member

calmh commented Jul 30, 2016

Merging master into the branch as necessary is preferred. It'll get squashed at merge time anyhow.

@calmh
Copy link
Member

calmh commented Jul 30, 2016

@st-jenkins add to whitelist

<img class="logo hidden-xs" src="assets/img/logo-horizontal.svg" height="32" width="117"/>
<img class="logo hidden visible-xs" src="assets/img/favicon-default.png" height="32"/>
</span>
<p class="navbar-text hidden-xs" ng-class="{'hidden-sm':upgradeInfo && upgradeInfo.newer}">{{thisDeviceName()}}</p>
<h1 class="navbar-text hidden-xs" ng-class="{'hidden-sm':upgradeInfo && upgradeInfo.newer}">{{thisDeviceName()}}</h1>
Copy link
Member

Choose a reason for hiding this comment

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

This change doesn't seem good though, it increases the size of the device name in the header drastically. Perhaps there is an aria tag that could be used instead to indicate it's headerness, or the styles need to be tweaked to not do this for h1-in-navbar.

screen shot 2016-07-30 at 10 08 02

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a huge deal if that isn't a heading, so I'll remove it. I meant to remove it, and forgot before pushing.

@AudriusButkevicius
Copy link
Member

Also, the html gains double spaces in some places, which I am not fully happy with.

@@ -514,9 +514,9 @@ <h4 class="panel-title">

<!-- Remote devices -->
<h3 translate>Remote Devices</h3>
<div class="panel-group" id="devices">
<div class="panel-group" id="devices"></div>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove this mistaken div closing tag I introduced.

<h3 translate>This Device</h3>
<div class="panel panel-default" ng-repeat="deviceCfg in [thisDevice()]">
<button class="btn panel-heading" data-toggle="collapse" data-target="#device-this">
<div class="panel panel-default" ng-repeat="deviceCfg in [thisDevice()]">
Copy link
Member

Choose a reason for hiding this comment

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

Double space here

@AudriusButkevicius
Copy link
Member

I'm fine to merge once last comments are addressed.

@AudriusButkevicius
Copy link
Member

@st-review merge

gui: Improve accessibility (fixes #3297)

skip-check: authors

@st-review
Copy link

👌 Merged as a8cd9d0. Thanks, @derekriemer!

st-review pushed a commit that referenced this pull request Jul 31, 2016
skip-check: authors

GitHub-Pull-Request: #3463
@st-review st-review closed this Jul 31, 2016
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Aug 1, 2017
@syncthing syncthing locked and limited conversation to collaborators Aug 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants