a11y and usability fixes #1134

akash07k · 2019-09-22T19:11:31Z

Lots of accessibility and usability fixes. #1133

akash07k · 2019-09-23T03:54:13Z

I don't know why, but when I'm expanding the sources, the sources are not displaying.

jtojnar

Thanks, I added some notes.

templates/home.phtml

jtojnar · 2019-09-22T21:30:32Z

templates/home.phtml

            <?php endif; ?>

-            <a id="nav-copyright" href="https://selfoss.aditu.de" target="_blank" rel="noopener noreferrer">selfoss <?= \F3::get('version'); ?></a>
+            <a id="nav-copyright" href="https://selfoss.aditu.de" target="_blank" rel="noopener noreferrer"><?= trim(\F3::get('html_title')); ?> <?= \F3::get('version'); ?></a>


I think this is an information about the version of the software so it should probably always be selfoss, rather than the name administrator chooses for a given installation. Though I would argue that user should not need to care about what program this really is so I would move it to the settings.

I think this is an information about the version of the software so it should probably always be selfoss, rather than the name administrator chooses for a given installation. Though I would argue that user should not need to care about what program this really is so I would move it to the settings.

Actually, I think for the better user experience, the name should be retrieved from the settings itself.
Also, since URL is unchanged and leads the user to the selfoss homepage, it should be fine. rest, it is your choice.

Also, since URL is unchanged and leads the user to the selfoss homepage, it should be fine.

Well this is precisely my point. I would expect the label to match the target URL so that user knows where will it lead before visiting it.

Also it is followed by a selfoss version number, which, I think, does not make sense when combined with a custom title.

I would just remove this link altogether, as it does not add much value while cluttering the sidebar.

Also, since URL is unchanged and leads the user to the selfoss homepage, it should be fine.

Well this is precisely my point. I would expect the label to match the target URL so that user knows where will it lead before visiting it.

Also it is followed by a selfoss version number, which, I think, does not make sense when combined with a custom title.

I would just remove this link altogether, as it does not add much value while cluttering the sidebar.

Yeah, removing is also a better option.
It will be great.
Also, when will you commit your changes so that I can pull them and work further?
I also want to introduce a heading level1 on th home page because user always looks for the main heading I.E: h1 for reading about the website/application and for the main content. Since main content is already present, where and with which content we should add h1?

.gitignore

templates/home.phtml

jtojnar · 2019-09-23T08:44:03Z

public/js/selfoss-events-entriestoolbar.js

@@ -49,10 +49,10 @@ selfoss.events.entriesToolbar = function(parent) {
        if (parent.find('.entry-toolbar').has('button.entry-share' + shares[0]).length == 0) {
            // add the share toolbar entries
            parent.find('.entry-smartphone-share button.entry-newwindow').after(selfoss.shares.buildLinks(shares, function(name) {
-                return '<button class="entry-share entry-share' + name + '" title="' + name + '"><img class="entry-share" title="' + name + '" src="images/' + name + '.png" height="16" width="16">' + name + '</button>';


Why use aria-haspopup? It opens a new tab/window, not a popup.

Why use aria-haspopup? It opens a new tab/window, not a popup.

You are right, but, since it behaves like link, either it should be changed to link, or if it is required to keep it button, then aria-haspopup is best bet for this. Since, normal buttons are not supposed to open any new tab/page

I will defer to you on this. Unfortunately, we cannot use link, as some sharers work in the background (e.g. copy to clipboard, share with operating system menu).

Maybe we could improve the button labels.

I will defer to you on this. Unfortunately, we cannot use link, as some sharers work in the background (e.g. copy to clipboard, share with operating system menu).

Maybe we could improve the button labels.

No problem, we can improve the labels.
Should I do something or you're already doing?

I will push the changes shortly.

templates/home.phtml

templates/item.phtml

templates/home.phtml

akash07k · 2019-09-23T12:13:09Z

So, should I wait for your commits in order to fix things further?

…

On 9/23/19, Jan Tojnar ***@***.***> wrote: jtojnar commented on this pull request. > <ul id="nav-tags"> <li class="active nav-tags-all" role="link" tabindex="0"><span role="listitem"><?= \F3::get('lang_alltags')?></span></li> </ul> - <h2 id="nav-sources-title" class="nav-sources-collapsed" tabindex="0" aria-expanded="false"><span role="link"><?= \F3::get('lang_sources') ?></span></h2> + <h2 id="nav-sources-title" class="nav-sources-collapsed" tabindex="0"><span role="link" aria-expanded="false"><?= \F3::get('lang_sources') ?></span></h2> Sure. Will update the scripts and the styles later today. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #1134 (comment)

akash07k · 2019-09-23T12:21:00Z

I don't know why, but when I'm expanding the sources, the sources are not displaying.

They are displaying now, but screen reader is not treating them as list still after changing the code.

jtojnar · 2019-09-23T12:42:36Z

Feel free to address what you like, I will be back from school in two or three hours and then will take a jab at it.

akash07k · 2019-09-23T14:58:50Z

Feel free to address what you like, I will be back from school in two or three hours and then will take a jab at it.

Sure, also, if you find anything wrong in the code, try to remove my changes.

akash07k · 2019-09-23T15:08:56Z

Ok, thanks

…

On 9/23/19, Jan Tojnar ***@***.***> wrote: jtojnar commented on this pull request. > @@ -49,10 +49,10 @@ selfoss.events.entriesToolbar = function(parent) { if (parent.find('.entry-toolbar').has('button.entry-share' + shares[0]).length == 0) { // add the share toolbar entries parent.find('.entry-smartphone-share button.entry-newwindow').after(selfoss.shares.buildLinks(shares, function(name) { - return '<button class="entry-share entry-share' + name + '" title="' + name + '"><img class="entry-share" title="' + name + '" src="images/' + name + '.png" height="16" width="16">' + name + '</button>'; I will push the changes shortly. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #1134 (comment)

jtojnar · 2019-09-23T16:09:41Z

Yup, they are in the headings above, which is why I suggested to use aria-labelledby. But if it is better for you to have the label be Filter by status/tag/source (a context that seeing users will get from the filters being together in the sidebar), it might be better to use a separate string.

Though, I would also be worried the screen reader will now read the label twice:

[Heading level 3] Tags
[List] Tags
…

akash07k · 2019-09-23T16:35:34Z

Yup, they are in the headings above, which is why I suggested to use aria-labelledby. But if it is better for you to have the label be Filter by status/tag/source (a context that seeing users will get from the filters being together in the sidebar), it might be better to use a separate string.

Though, I would also be worried the screen reader will now read the label twice:
[Heading level 3] Tags
[List] Tags
…

Pushed the changes. removed the duplicated strings and used aria-labeledby
No. there's no issue, for example, JAWS reads like this:
Heading level 2 tags.
Tags list of 19 items.
Which is fine.

jtojnar · 2019-09-23T21:47:25Z

Rebased onto master.
Dropped the db patches since that is separate issue (Log database driver name when estabilishing a connection #944)
Converted the <span>s with roles to proper HTML elements, fixed the JavaScript and styles.
Specialized the login form autocomplete attributes.
Removed the branding link in the sidebar.

I left out the aria-expanded on .entry-title, as that one is non-trivial to fix. We will need the closing/opening methods as @niol suggested in #1127 (comment).

jtojnar · 2019-09-24T12:08:30Z

I am looking at adding the buttons to the entry titles some more and there is an issue: micro-articles such as tweets can contain links inside the titles. For that reason we cannot really make the whole title clickable. I am not sure how to represent that properly.

jtojnar · 2019-09-24T13:35:10Z

For now, I have made the .entry-title a role=link and moved aria-expanded there. But I am not sure how will it behave with tweets containing links in title. Here is an example of such tweet body, could you check how it behaves with screen readers https://jsfiddle.net/2hxLrgjt/1/show.

I have also rebased this onto master and it is ready to be merged, pending a review from @niol regarding the latest commit.

niol

Looks good

templates/tag.phtml

akash07k · 2019-09-24T15:16:30Z

For now, I have made the .entry-title a role=link and moved aria-expanded there. But I am not sure how will it behave with tweets containing links in title. Here is an example of such tweet body, could you check how it behaves with screen readers https://jsfiddle.net/2hxLrgjt/1/show.

I have also rebased this onto master and it is ready to be merged, pending a review from @niol regarding the latest commit.

I've just checked your example, no problem, screen readers are recognising every link as individual link and there's no problem with the readability also.
I feel that since screen readers are not having any problem with the links contained in the title, can't we just make the entry title button as before? I don't think that it will cause problems.

akash07k · 2019-09-24T15:19:54Z

I am looking at adding the buttons to the entry titles some more and there is an issue: micro-articles such as tweets can contain links inside the titles. For that reason we cannot really make the whole title clickable. I am not sure how to represent that properly.

If we'll make the whole title clickable, won't the user be able to click the individual links contained in the title?

jtojnar · 2019-09-24T16:36:13Z

I've just checked your example, no problem, screen readers are recognising every link as individual link and there's no problem with the readability also.

That’s great.

I feel that since screen readers are not having any problem with the links contained in the title, can't we just make the entry title button as before? I don't think that it will cause problems.

Unfortunately, HTML does not allow interactive elements inside links or buttons, so we cannot use the native elements. I would say the link role fits better than the button, since expanding the item also changes the URL hash to point to that page.

If we'll make the whole title clickable, won't the user be able to click the individual links contained in the title?

Not if we use native HTML elements.

akash07k · 2019-09-24T16:39:38Z

oh, ok. fine. let's stick with role="link" as of now. if there will be any issue, then we'll look forward for role="button"

…

On 9/24/19, Jan Tojnar ***@***.***> wrote: > I've just checked your example, no problem, screen readers are recognising > every link as individual link and there's no problem with the readability > also. That’s great. > I feel that since screen readers are not having any problem with the links > contained in the title, can't we just make the entry title button as > before? I don't think that it will cause problems. Unfortunately, HTML does not allow interactive elements inside [links](https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-a-element:concept-element-content-model) or [buttons](https://html.spec.whatwg.org/#the-button-element:concept-element-content-model), so we cannot use the native elements. I would say the `link` role fits better than the `button`, since expanding the item also changes the URL hash to point to that page. > If we'll make the whole title clickable, won't the user be able to click > the individual links contained in the title? Not if we use native HTML elements. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #1134 (comment)

akash07k · 2019-09-24T16:41:43Z

Is there any way to identify that which filter, source and tag is currently active and put aria-current on it?

…

On 9/24/19, Akash Kakkar ***@***.***> wrote: oh, ok. fine. let's stick with role="link" as of now. if there will be any issue, then we'll look forward for role="button" On 9/24/19, Jan Tojnar ***@***.***> wrote: >> I've just checked your example, no problem, screen readers are >> recognising >> every link as individual link and there's no problem with the >> readability >> also. > > That’s great. > >> I feel that since screen readers are not having any problem with the >> links >> contained in the title, can't we just make the entry title button as >> before? I don't think that it will cause problems. > > Unfortunately, HTML does not allow interactive elements inside > [links](https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-a-element:concept-element-content-model) > or > [buttons](https://html.spec.whatwg.org/#the-button-element:concept-element-content-model), > so we cannot use the native elements. I would say the `link` role fits > better than the `button`, since expanding the item also changes the URL > hash > to point to that page. > >> If we'll make the whole title clickable, won't the user be able to click >> the individual links contained in the title? > > Not if we use native HTML elements. > > -- > You are receiving this because you authored the thread. > Reply to this email directly or view it on GitHub: > #1134 (comment)

jtojnar · 2019-09-24T17:01:03Z

Good idea. Will do that.

akash07k · 2019-09-24T17:05:30Z

Thanks bro.

…

On 9/24/19, Jan Tojnar ***@***.***> wrote: Good idea. Will do that. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #1134 (comment)

akash07k · 2019-09-24T17:05:56Z

I'm waiting for the pagination implementation and will try to fix some things if required in that as well

…

On 9/24/19, Akash Kakkar ***@***.***> wrote: Thanks bro. On 9/24/19, Jan Tojnar ***@***.***> wrote: > Good idea. Will do that. > > -- > You are receiving this because you authored the thread. > Reply to this email directly or view it on GitHub: > #1134 (comment)

jtojnar · 2019-09-24T21:39:51Z

@niol I refactored entry selection as well. Could you please take a look at it as well?

jtojnar · 2019-09-24T22:18:30Z

I think the aria-current for navigation will be better handled in #1064

akash07k · 2019-09-25T03:35:49Z

Thanks for your commits bro, it's working properly. No issue so far.

…

On 9/25/19, Akash Kakkar ***@***.***> wrote: Thanks a lot for this, but now aria-expanded is not working for the entry titles. When we are clicking on an entry title, expanded="true" is not happening. can you please look at this? On 9/25/19, Jan Tojnar ***@***.***> wrote: > I think the `aria-current` for navigation will be better handled in > #1064 > > -- > You are receiving this because you authored the thread. > Reply to this email directly or view it on GitHub: > #1134 (comment)

jtojnar · 2019-09-25T11:13:51Z

I have pushed the composer updates to master as there do not seem to be any breaking changes.

As for the jQuery update, the security vulnerabilities do not affect us and there are many breaking changes regarding to Deferred, which will need to be manually checked. And we should probably switch to A+-style promises throughout the codebase.

The remaining deprecations and breaking changes should be addressed in 8e991c4, except for bind() deprecation. That is better left for a separate PR.

Added autocomplete to the user login form and made the title of the login form to retrieve from the configurations

Added the aria-label explicitly to some controls, because titles are treated as tooltip by the screen readers and some screen readers do not announce the tooltips. In this case user will not be able to understand that what the button does. Aria-label is a perfect fix for that

Users should not need to care, what version of selfoss are they running. Also cleaned up the sidebar button bar slightly, and fixed a bug where refresh button was always shown.

The headings for "filters", "tags" and "sources" are clickable like a link so, added the link role as well along with the heading. There was no need of aria-haspopup attribute, so removed all the references. Also, Filters, Tags etc are defined as a list but role="link" added to the list item because they work like a link, thus, screen readers do not treat them as a list, thus, added the role="listitem" also in the span so that screen reader properly recognise them as both list and link. Co-Authored-By: Jan Tojnar <[email protected]>

All the stories should be in h3 not in h2, so fixed that as well.

Removed maximum-scale and changed the user-scalable to user-scalable="yes". Setting the maximum-scale=1 and user-scalable=no makes the zooming useless and difficult for the low vision users. Please refer: https://a11yproject.com/posts/never-use-maximum-scale/ Changed some more things so that application can retrieve the title/name of the app from the configurations.

Fixed the zooming for OPML and hasspassword templates as well. Also added the main landmark.

Fixed some issues in sources.phtml template. made the favicon hidden by aria-hidden="true" because alt="" is not a efficient practise.

Added the access keys for the common controls for the better user experience and also added some aria attributes.

For easier and effective navigation…

Also wrap the contents of `.entry-title` with `span[role=link].entry-title-link` which is now the holder for the `aria-expanded` attribute. This will improve the accessibility of the entry page. Unfortunately, it might cause some issues with tweets, that can contain links in the title. Also change the focus target when navigating using keyboard to that link, since it is tabindexed, unlike the icon, used previously.

Previously, we had a separate #fullscreen-entry element, where we copied the body of entry when opened on mobile. Now we are using CSS to make the original entry occupy the full viewport.

niol

What's in master/more-a11y seems to work with no obvious problem.

jtojnar · 2019-09-25T14:31:45Z

Thanks for all the help.

akash07k · 2019-09-25T15:10:14Z

Thanks for all the help.

Thanks a lot to you too and @niol As well. I'm really liking contributing with you people and I'm very happy with all your cooperation.
You guys are really great.
Also, if I have some other fix for a11y, should I open a new pull requestor reopen it with my changes? What do you recommend?

jtojnar · 2019-09-25T15:14:20Z

Please open a new pull request. GitHub does not allow re-opening PRs that were merged anyway.

Also please try to split commits that do several (even small) different things on different components into multiple commits. And use git commit --fixup when you need to fix something in a previous commit – that will allow us to easily squash typo commits before the pull request is merged.

akash07k · 2019-09-25T15:41:42Z

Please open a new pull request. GitHub does not allow re-opening PRs that were merged anyway.

Also please try to split commits that do several (even small) different things on different components into multiple commits. And use git commit --fixup when you need to fix something in a previous commit – that will allow us to easily squash typo commits before the pull request is merged.

Sure!

jtojnar · 2020-09-27T00:06:29Z

Hi @akash07k. Previously, it was not clear if selfoss was licensed under GPL 3 only, or also any later version. Could you clarify whether you are fine with licensing your contribution under GPL-3-or-later?

Thanks again and sorry for the confusion.

akash07k · 2020-09-27T11:58:24Z

@jtojnar
Yes, absolutely buddy.
No problem at all

Hi @akash07k. Previously, it was not clear if selfoss was licensed under GPL 3 only, or also any later version. Could you clarify whether you are fine with licensing your contribution under GPL-3-or-later?

Thanks again and sorry for the confusion.

jtojnar reviewed Sep 23, 2019

View reviewed changes

akash07k changed the title ~~Accessibility fixes~~ a11y and usability fixes Sep 23, 2019

jtojnar mentioned this pull request Sep 24, 2019

Horizontal scrolling instead of single column view #1127

Open

niol approved these changes Sep 24, 2019

View reviewed changes

templates/tag.phtml Outdated Show resolved Hide resolved

templates/tag.phtml Outdated Show resolved Hide resolved

jtojnar requested a review from niol September 24, 2019 21:39

Akash Kakkar added 2 commits September 25, 2019 13:14

a11y: improve login form

8459dd9

Added autocomplete to the user login form and made the title of the login form to retrieve from the configurations

jtojnar and others added 16 commits September 25, 2019 13:14

client: Drop branding from the sidebar

f78d9fd

Users should not need to care, what version of selfoss are they running. Also cleaned up the sidebar button bar slightly, and fixed a bug where refresh button was always shown.

a11y: many more small a11y fixes

fb98d92

All the stories should be in h3 not in h2, so fixed that as well.

a11y: Convert entry toolbars to lists

3ad1fdb

a11y: improve metadata on OPML & hashpashword pages

7ed6303

Fixed the zooming for OPML and hasspassword templates as well. Also added the main landmark.

a11y: Improve sources page

6a09b57

Fixed some issues in sources.phtml template. made the favicon hidden by aria-hidden="true" because alt="" is not a efficient practise.

a11y: Add accesskeys

4c9b6be

Added the access keys for the common controls for the better user experience and also added some aria attributes.

a11y: Added autocomplete=new-password to the hashpassword template

504a8c9

a11y: add ARIA labels to nav lists

fd68a81

a11y: Add label for the entry action list

492c4be

a11y: Add access keys for the 'source form'

fc22251

For easier and effective navigation…

a11y: Add some more access keys

f87549e

client: refactor entry selection

d4e362d

Previously, we had a separate #fullscreen-entry element, where we copied the body of entry when opened on mobile. Now we are using CSS to make the original entry occupy the full viewport.

a11y: add aria-current to the selected item

d089773

niol reviewed Sep 25, 2019

View reviewed changes

jtojnar merged commit d089773 into fossar:master Sep 25, 2019

jtojnar mentioned this pull request Oct 15, 2019

Bump jQuery version #1029

Closed

jtojnar mentioned this pull request Jan 7, 2022

A couple minor things #1305

Closed

jtojnar mentioned this pull request Oct 12, 2022

Add the version number to the "add sources" page #1374

Open

a11y and usability fixes #1134

a11y and usability fixes #1134

Conversation

akash07k commented Sep 22, 2019 • edited Loading

akash07k commented Sep 23, 2019

jtojnar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akash07k commented Sep 23, 2019 via email

akash07k commented Sep 23, 2019

jtojnar commented Sep 23, 2019

akash07k commented Sep 23, 2019

akash07k commented Sep 23, 2019 via email

jtojnar commented Sep 23, 2019

akash07k commented Sep 23, 2019

jtojnar commented Sep 23, 2019 • edited Loading

jtojnar commented Sep 24, 2019

jtojnar commented Sep 24, 2019

niol left a comment

Choose a reason for hiding this comment

akash07k commented Sep 24, 2019

akash07k commented Sep 24, 2019

jtojnar commented Sep 24, 2019

akash07k commented Sep 24, 2019 via email

akash07k commented Sep 24, 2019 via email

jtojnar commented Sep 24, 2019

akash07k commented Sep 24, 2019 via email

akash07k commented Sep 24, 2019 via email

jtojnar commented Sep 24, 2019

jtojnar commented Sep 24, 2019

akash07k commented Sep 25, 2019 via email

jtojnar commented Sep 25, 2019

niol left a comment

Choose a reason for hiding this comment

jtojnar commented Sep 25, 2019

akash07k commented Sep 25, 2019

jtojnar commented Sep 25, 2019

akash07k commented Sep 25, 2019

jtojnar commented Sep 27, 2020

akash07k commented Sep 27, 2020

akash07k commented Sep 22, 2019 •

edited

Loading

jtojnar commented Sep 23, 2019 •

edited

Loading