[#20682] - Button cleanup
- Closed
- 4 Oct 2018
- Medium
- Build: staging
- # 20682
- Diff
- SharkyKZ:Buttons_3x
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
This cleans up mainly modal-related buttons. Most notable change is restoration of <button> element which was removed in PRs #11787, #14483, #16156. The original issue #11765 was caused by missing type attribute in user field layout. Although switching to <a> tag fixed the issue, it wasn't the right way to do it.
For future reference:
Use <button> element in favor of <a> for button functionality whenever possible (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_button_role).
Always add type attribute (most browsers default to type="submit" without it).
button is not a valid value for type attribute on <a> (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-type).
Don't use aria-hidden="true" on buttons containing readable text. Use it on non-readable elements like icons.
Testing Instructions
- Code review.
- Check that buttons work correctly.
- Confirm that you cannot reproduce issue #11765.
Documentation Changes Required
No.
Votes
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_associations com_banners com_categories com_contact com_content com_fields com_media com_menus com_messages com_modules com_newsfeeds com_redirect com_tags com_templates |
| Labels |
Added:
?
|
||
Searching the entire codebase for 'a class="btn' reveals a lot more that might need to be checked
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20682.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20682.
@brianteeman This PR cleans up mainly modal-related buttons.
@brianteeman can you please retest?
I did a search on <button class= and found more to move type before class.
There are also more buttons with type missing. But keep that for a separate PR.
| Category | Administration com_associations com_banners com_categories com_contact com_content com_fields com_media com_menus com_messages com_modules com_newsfeeds com_redirect com_tags com_templates | ⇒ | Repository Administration com_associations com_banners com_categories com_contact com_content com_fields com_media com_menus com_messages com_modules com_newsfeeds com_redirect com_tags com_templates |
Why is Security.md in this PR?
| Category | Administration com_associations com_banners com_categories com_contact com_content com_fields com_media com_menus com_messages com_modules com_newsfeeds com_redirect com_tags com_templates Repository | ⇒ | Unit Tests Repository Administration com_admin |
| Labels |
Added:
?
|
||
| Category | Administration Repository Unit Tests com_admin | ⇒ | Repository Administration com_associations com_banners com_categories com_contact com_content com_fields com_media com_menus com_messages com_modules com_newsfeeds com_redirect com_tags com_templates |
EOL issue. Closing this now. Going to break it down into smaller pieces.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-10-04 06:51:42 |
| Closed_By | ⇒ | SharkyKZ | |
| Labels |
Removed:
?
|
||
I have tested this item✅ successfully on a2aa527
tested all the modals I could think of
Tested to make sure the original bug had not come back
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20682.