-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add skip navigation / skip to content links for accessibility #10004
Conversation
Ooh, very clever!!! 👍 |
We should create an entry related to the accessibility in the docs! |
54d5a2e
to
c388196
Compare
Codecov Report
@@ Coverage Diff @@
## master #10004 +/- ##
============================================
+ Coverage 31.7% 31.7% +<.01%
+ Complexity 26017 26014 -3
============================================
Files 1661 1660 -1
Lines 96171 96153 -18
Branches 1290 1290
============================================
Hits 30490 30490
+ Misses 65681 65663 -18
|
@skjnldsv yeah, a section in the design docs where we recommend testing with WAVE, by keyboard, Lighthouse, or Firefox accessibility inspector :) |
.buildconfig
Outdated
app-id= | ||
postbuild= | ||
prebuild= | ||
default=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this?
core/fonts/OFL.txt
Outdated
INCLUDING ANY GENERAL, SPECIAL, INDIRECT, INCIDENTAL, OR CONSEQUENTIAL | ||
DAMAGES, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
FROM, OUT OF THE USE OR INABILITY TO USE THE FONT SOFTWARE OR FROM | ||
OTHER DEALINGS IN THE FONT SOFTWARE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that those fonts are not needed for this PR, right?
@MorrisJobke sorry – miscommitted, will fix. |
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
c388196
to
de5d5ee
Compare
@MorrisJobke fixed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessibility bits approved, rest is yours to clean up. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
It is probably too late to comment it there, but should not the texts be put through the translation machinery? The wording is the one i saw everywhere else, however, translating them could be a good idea. |
That is an excellent point! |
@tyrylu oops, good catch! Will fix tomorrow :) or if you want to open a pull request for it, you are very welcome! :) |
Follow-up PR for translation at #10019 :) |
Add "skip navigation" links according to https://webaim.org/techniques/skipnav/
First tab gets you directly to the main #app-content (skipping both header and left sidebar):
Second tab gets you to #app-navigation (left sidebar of the app):
After that it continues in the header through the apps as usual.
Please review @nextcloud/accessibility @nextcloud/designers :)