Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

OcButton: Change default type of buttons to button #2009

Merged
merged 1 commit into from
Mar 7, 2022

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Mar 7, 2022

Description

This changes the default type of OcButton to button.
The default type for <button> elements is submit. As we use it all over the place now for a11y reasons, we have lots and lots of submit buttons everywhere, which is most likely not what we want.

Related Issue

Could not find an open ticket for this issue.

Motivation and Context

I recently noticed that pressing enter in one of the text inputs in the public link editing view opened the datepicker instead of saving the link:

liveview-2022-03-07_22.14.14.mp4

It was @fschade (💖 ) who pointed me towards the root of the issue:
pressing enter submits the form, which automatically clicks the first (at least in chromium) button which is not of type "button". In this case this happened to be the button that opens the datepicker.
Now I would argue that more often than not buttons should not behave as submit buttons and we should instead mark submit buttons explicitly. As there are only a few occurences of <form in oC Web, this change should not be too invasive...

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation added/updated

Open tasks:

  • ...

@update-docs
Copy link

update-docs bot commented Mar 7, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@dschmidt dschmidt force-pushed the make_oc_buttons_of_type_button_by_default branch 2 times, most recently from 3a56265 to bc3e4ce Compare March 7, 2022 21:38
@dschmidt dschmidt changed the title Make OcButton of type button by default OcButton: Change default type of buttons to button Mar 7, 2022
@dschmidt dschmidt force-pushed the make_oc_buttons_of_type_button_by_default branch from bc3e4ce to d6aaedf Compare March 7, 2022 21:41
@sonarcloud
Copy link

sonarcloud bot commented Mar 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

😍

@dschmidt dschmidt merged commit fe4e713 into master Mar 7, 2022
@delete-merged-branch delete-merged-branch bot deleted the make_oc_buttons_of_type_button_by_default branch March 7, 2022 21:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants