Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add alchemy-button web component #2621

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Add alchemy-button web component #2621

merged 2 commits into from
Nov 28, 2023

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Nov 24, 2023

What is this pull request for?

This allows to remove all the Alchemy.Buttons state management code we have right now and let the browser do it for us.

This DOES not work in Safari as they do not support extending existing HTML elements, but this is ok in this case. Maybe we will provide a polyfill someday or just do not support this feature in Safari.

In order to update forms not using the alchemy_form_for helper, replace all usages of data-alchemy-button with is="alchemy-button".

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@tvdeyen tvdeyen added this to the 7.1 milestone Nov 24, 2023
This allows to remove all the Alchemy.Buttons state management
code we have right now and let the browser do it for us.

This DOES not work in Safari as they do not support extending
existing HTML elements, but this is ok in this case. Maybe we will
provide a polyfill someday or just do not support this feature in Safari.

In order to update forms not using the `alchemy_form_form` helper,
replace all usages of `data-alchemy-button` with `is="alchemy-button"`.
@tvdeyen tvdeyen force-pushed the alchemy-button-component branch from c682ff0 to 370aa8a Compare November 24, 2023 21:07
Jest logs the error that is thrown purposely in this spec.
@tvdeyen tvdeyen requested a review from a team November 27, 2023 19:44
@tvdeyen tvdeyen mentioned this pull request Nov 27, 2023
3 tasks
Copy link
Contributor

@sascha-karnatz sascha-karnatz left a comment

Choose a reason for hiding this comment

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

This will not work in Safari, but it does not block Safari user. We have to polyfil the is - attributes in the future.

const submit = new Event("submit", { bubbles: true })
button.form.dispatchEvent(submit)

expect(button.getAttribute("disabled")).toEqual("disabled")
Copy link
Contributor

Choose a reason for hiding this comment

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

You could split that into different describe and it - functions. Jest is pretty fast it would desribe, what are you going to test.

@tvdeyen tvdeyen merged commit a43e3d7 into main Nov 28, 2023
56 checks passed
@tvdeyen tvdeyen deleted the alchemy-button-component branch November 28, 2023 07:56
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.

2 participants