Skip to content

[4.0][a11y] 'Cancel' button changes from an anchor tag to a button tag (profile edit)#24007

Closed
tarot-ray wants to merge 7 commits intojoomla:4.0-devfrom
tarot-ray:patch-4
Closed

[4.0][a11y] 'Cancel' button changes from an anchor tag to a button tag (profile edit)#24007
tarot-ray wants to merge 7 commits intojoomla:4.0-devfrom
tarot-ray:patch-4

Conversation

@tarot-ray
Copy link
Copy Markdown
Contributor

For A11Y reasons, this should be a button tag, not a styled anchor tag.

Pull Request for Issue # .

Summary of Changes

'Cancel' button is changed to a button tag from an anchor tag, to improved A11Y

Testing Instructions

Check that the 'Cancel' button is seen correctly by a screenreader.

Expected result

Improved A11Y.

Actual result

Documentation Changes Required

For A11Y reasons, this should be a button tag, not a styled anchor tag.
@SharkyKZ
Copy link
Copy Markdown
Contributor

This doesn't work. Button with type="button" needs some JS code to do anything. href is not a valid attribute for element button. If this really needs to be a button it should submit the form with cancel task.

@infograf768 infograf768 changed the title [4.0][a11y] 'Cancel' button changes from an anchor tag to a button tag [4.0][a11y] 'Cancel' button changes from an anchor tag to a button tag (profile edit) Feb 25, 2019
Copy link
Copy Markdown
Contributor

@wilsonge wilsonge left a comment

Choose a reason for hiding this comment

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

As per the comments

@wilsonge
Copy link
Copy Markdown
Contributor

I have tested this item 🔴 unsuccessfully on 9ce88b6


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

@wilsonge
Copy link
Copy Markdown
Contributor

You still need to address the comments from @SharkyKZ

@wilsonge
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 0b3c29f


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

@wilsonge
Copy link
Copy Markdown
Contributor

@tarot-ray almost there - in the latest commit you've accidentally removed the classes again :)

@zwiastunsw
Copy link
Copy Markdown
Contributor

zwiastunsw commented Feb 27, 2019

@tarot-ray: Add: class="btn btn-danger"

@tarot-ray
Copy link
Copy Markdown
Contributor Author

So, further changes have been made to the PR to make the line absolutely correct and perfectly A11Y compatible. Please, review.

@zwiastunsw
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 74c891e


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

@wilsonge
Copy link
Copy Markdown
Contributor

LGTM!

@wilsonge
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 30dd62a


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

@wilsonge
Copy link
Copy Markdown
Contributor

Just whitespace changes between the last two tests so RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 27, 2019
@SharkyKZ
Copy link
Copy Markdown
Contributor

What exactly is the point of changing anchor to a button that does exactly the same but with JS dependency?

@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented Feb 27, 2019

@zwiastunsw Actually in this specific case thinking about it - I think a link was the correct choice because this is a redirect to another page. From http://web-accessibility.carnegiemuseums.org/content/buttons/

Navigation: If you want a user to navigate to a new page or to a different target on the same page, use an anchor element <a>.

Basically buttons should be used if you're doing stuff on the same page and links for moving to another page

@zwiastunsw
Copy link
Copy Markdown
Contributor

zwiastunsw commented Feb 27, 2019

@SharkyKZ , @wilsonge :
It's a bit more complicated. In this case, Cancel will actually redirect to another page. But the essence of this control is to cancel the action and not to move user to another page.
In this and other similar cases, Cancel cancels actions you have performed.
The command also speaks: Cancel, not Go to.
This is not just a tag problem. If you use a tag, make it look like a link and not like a button. Because it's confusing for keyboard users. I run the link with the Enter key, the button I run with the Enter or Space keys. If you activating link with the spacebar causes the page to scroll and not running the link.

@SharkyKZ
Copy link
Copy Markdown
Contributor

SharkyKZ commented Mar 4, 2019

See #24088 for potential alternative please.

@ghost
Copy link
Copy Markdown

ghost commented Mar 5, 2019

@wilsonge can this PR be closed?

@wilsonge
Copy link
Copy Markdown
Contributor

I've merged #24088 so closing this

@wilsonge wilsonge closed this Mar 21, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 21, 2019
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.

6 participants