? ? Pending

User tests: Successful: Unsuccessful:

avatar chmst
chmst
30 Jan 2019

Summary of Changes

Action Buttons for featuring articles for example in com_content/articles are Links, not Buttons.
This PR

  • changes the a tag to button tag
  • removes popver and replaces it by a sr-only class
  • makes a separate column in the articles list
  • enables sorting the list by featured articles - see also #23497

Testing Instructions

Apply the patch and see that the featured, unfeatured function works as before.

featured-column-articles

NOTE:
When activating one featured button, the corresponding checkbox and the Status Change Dropdown are activated for a moment. #23717

Documentation Changes Required

new screen

avatar chmst chmst - open - 30 Jan 2019
avatar chmst chmst - change - 30 Jan 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Jan 2019
Category Administration com_content Layout
avatar chmst chmst - change - 30 Jan 2019
The description was changed
avatar chmst chmst - edited - 30 Jan 2019
avatar chmst chmst - change - 30 Jan 2019
The description was changed
avatar chmst chmst - edited - 30 Jan 2019
avatar brianteeman
brianteeman - comment - 30 Jan 2019

Almost perfect. woohoo

  1. The td for the featured header should be a th
  2. Needs some sort of onhover visual guide to indicate to a mouse user that this is clickable
avatar chmst chmst - change - 30 Jan 2019
Labels Added: ?
avatar zwiastunsw
zwiastunsw - comment - 31 Jan 2019
  1. It is a good idea to create a new column and separate the status and feaured buttons.
  2. I wonder if a simple button is a good choice in this case. Shouldn't it be a toggle button?. Here we have a switchover of the position status. @dgrammatiko - What do you think about it? Maybe the toggle button change should be in a separate PR?
  3. I am not a programmer. It seems to me, however, that the attribute href="javascript://" is not allowed for the button tag. It causes the button to work as a link.
  4. The action should not be followed by a change of context. The focus should remain on the button. When I use a screen reader, the focus jumps to a modal window with a status change message and then returns to the button (it is OK). But when I use only the keyboard (without the screen reader), the focus jumps to the modal window and remains at the top of the page when the window is turned off.
  5. If the button is a toggle button,then it has an aria-pressed=”true/false” state to let the screen reader user know when the button is pressed. Then you don't need to display a modal window with a message and move the focus to the modal window.
avatar dgrammatiko
dgrammatiko - comment - 31 Jan 2019

Shouldn't it be a toggle button?. Here we have a switchover of the position status.

In sort I think no. The reason is that the effect is not realtime (you submitting a form each time you're clicking on any of these). If you change the front end logic so the form submission will happen with an Ajax then a switcher will be fine but with the current code the current approach is fine

avatar zwiastunsw
zwiastunsw - comment - 31 Jan 2019

@dgrammatiko : OK, thx.
I understand that you now need to correct the code (remove the href attribute) and solve the focus problem. But I don't have an idea here.

Needs some sort of onhover visual guide to indicate to a mouse user that this is clickable

Add cursor: pointer in CSS?

avatar dgrammatiko
dgrammatiko - comment - 31 Jan 2019

@chmst @zwiastunsw here is some (untested) code:

		<?php if (empty($disabled)): ?>
				onclick="return false;"
			<?php endif; ?>

			<?php if(!empty($task) && empty($disabled)): ?>
				onclick="return Joomla.listItemTask('<?php echo $checkboxName . $this->escape($row ?? ''); ?>', '<?php echo $this->escape(isset($task) ? $taskPrefix . $task : ''); ?>')"
                         <?php endif; ?>
avatar brianteeman
brianteeman - comment - 31 Jan 2019

The action should not be followed by a change of context. The focus should remain on the button. When I use a screen reader, the focus jumps to a modal window with a status change message and then returns to the button (it is OK). But when I use only the keyboard (without the screen reader), the focus jumps to the modal window and remains at the top of the page when the window is turned off.

You are absolutely correct. But its really beyond the scope of this PR to address this as it is a bigger issue that I raised before. The information that the feature has been changed that takes the focus has incorrectly got a role of alert. It shouldnt be an alert as an alert will always "steal" the focus.

Needs some sort of onhover visual guide to indicate to a mouse user that this is clickable

Add cursor: pointer in CSS?

Either that or an outline in CSS

avatar chmst
chmst - comment - 31 Jan 2019

Thank you for advices. I have prepared a new commit.

  • No toggle button at the moment
  • @dgrammatiko i've tested you code but "retun false" would block the change from unfeatured to featured if I understand this right.
  • cursor pointer is a good idea I will do some css for cursor and hover later

I have changed:
If a user is not allowed to change the state, only an icon is displayed, it is not a button.

featured-disabled

About the message and the focus - as @brianteeman says, it is out of scope of this PR

avatar zwiastunsw
zwiastunsw - comment - 1 Feb 2019

I propose to add to the tag button title attribute calling the current status

avatar chmst
chmst - comment - 2 Feb 2019

I added a title instead of sr-only.
I'd like to make another PR for Cursor pointer and other css actions, this should be done in a own PR.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23718.

avatar zwiastunsw zwiastunsw - test_item - 2 Feb 2019 - Tested successfully
avatar chmst chmst - change - 2 Feb 2019
Title
[4.0][POC]Make ActionButton Featured accessible
[4.0] [a11y] Make ActionButton Featured accessible
avatar chmst chmst - edited - 2 Feb 2019
avatar zwiastunsw
zwiastunsw - comment - 2 Feb 2019

I'm sory. My mistake. I am withdrawing my positive test.
Please restore the label for screen readers.
The title attribute is not announced by screen readers.
Adding the title attribute means that the button also has a readable name for sighted users (e.g. when they point the icon with the mouse). The code should contain and attribute title and label for screen reader users (sr-only)

avatar chmst
chmst - comment - 2 Feb 2019

Please retest. Thank you for your patience. If this is all right, I will change other list views in the same way.

avatar zwiastunsw
zwiastunsw - comment - 2 Feb 2019

I suggested using the title attribute, because without this attribute the meaning of the button may not be understood by sighted users. Perhaps there is a better way to do this. I don't know. I remember what he said Karl Groves:

If I found a genie and it gave me one wish, it would be that web developers stopped using the title attribute.

(Karl Groves, December 15, 2017).

Now some screen readers will double the label. However, I think this is more accessible than without the title attribute. If it was a toggle buton, there would be no such problem :).
See: The Trials and Tribulations of the Title Attribute
That is why I am testing successfully.

avatar zwiastunsw
zwiastunsw - comment - 2 Feb 2019

I have tested this item successfully on 390f47b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23718.

avatar zwiastunsw zwiastunsw - test_item - 2 Feb 2019 - Tested successfully
avatar chmst
chmst - comment - 2 Feb 2019

Thank you for testing. The title attribute is ugly. I have isolated the column for Featured, hoping that the meaning of the button is evident for sighted administrators. But if it is necessary ...

avatar zwiastunsw
zwiastunsw - comment - 2 Feb 2019

@brianteeman What is your opinion. Because @chmst rightly says that the name of the column should be enough. Maybe I mixed up unnecessarily...

avatar brianteeman
brianteeman - comment - 2 Feb 2019

I agree 100% with @chmst

avatar brianteeman brianteeman - test_item - 2 Feb 2019 - Tested successfully
avatar brianteeman brianteeman - test_item - 2 Feb 2019 - Not tested
avatar zwiastunsw
zwiastunsw - comment - 2 Feb 2019

I have tested this item successfully on c3772f9


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23718.

avatar zwiastunsw zwiastunsw - test_item - 2 Feb 2019 - Tested successfully
avatar brianteeman
brianteeman - comment - 2 Feb 2019

I have tested this item successfully on c3772f9


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23718.

avatar brianteeman brianteeman - test_item - 2 Feb 2019 - Tested successfully
avatar Quy Quy - change - 2 Feb 2019
Status Pending Ready to Commit
avatar Quy
Quy - comment - 2 Feb 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23718.

avatar wilsonge wilsonge - change - 2 Feb 2019
Labels Added: ?
avatar wilsonge wilsonge - change - 3 Feb 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-02-03 22:59:28
Closed_By wilsonge
avatar wilsonge wilsonge - close - 3 Feb 2019
avatar wilsonge wilsonge - merge - 3 Feb 2019
avatar wilsonge
wilsonge - comment - 3 Feb 2019

Thanks!

Add a Comment

Login with GitHub to post a comment