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

fix(donate-block): allow spaces in button label #1479

Merged
merged 3 commits into from
Jun 21, 2023

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Jun 19, 2023

All Submissions:

Changes proposed in this Pull Request:

Fixes a minor but annoying bug in which the RichText component for the submit button in the editor for Donate blocks doesn't allow spaces if it's nested inside a button element. This fix changes the button element to a div with the same styles, in the editor only. See also Automattic/newspack-newsletters#1202

Closes 1204751246381643/1204483195828260.

How to test the changes in this Pull Request:

  1. On master, add a Donate block to a post or page.
  2. In both Frequency and Tiers layout options, observe that you can edit the submit button labels but can't add spaces in either case.
  3. Check out this branch, repeat step 2 and confirm that you can now add spaces, and that both editor and front-end styles and behavior are otherwise unchanged.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo requested a review from a team as a code owner June 19, 2023 22:42
@dkoo dkoo self-assigned this Jun 19, 2023
@dkoo dkoo added [Status] Needs Review bug Something isn't working labels Jun 19, 2023
Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

The changes work fine in the front-end but it broke the layout of the button for me in the editor
Captura de tela de 2023-06-21 15-12-03

There's an extra padding and and a border

@dkoo
Copy link
Contributor Author

dkoo commented Jun 21, 2023

Whoops, I got too fancy trying to inherit existing styles using an existing class name. But that class name has different styles depending on the "style" selected for the block. 250115f should make things consistent across all styles and layouts.

@dkoo dkoo requested a review from leogermani June 21, 2023 21:29
@dkoo dkoo merged commit 96adc82 into master Jun 21, 2023
matticbot pushed a commit that referenced this pull request Jun 22, 2023
# [1.71.0-alpha.1](v1.70.0...v1.71.0-alpha.1) (2023-06-22)

### Bug Fixes

* **donate-block:** allow spaces in button label ([#1479](#1479)) ([96adc82](96adc82))

### Features

* display purchase details in checkout modal when taxes are enabled ([#1480](#1480)) ([b69f8f3](b69f8f3))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.71.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Jun 29, 2023
# [1.72.0-alpha.1](v1.71.0...v1.72.0-alpha.1) (2023-06-29)

### Bug Fixes

* **donate-block:** allow spaces in button label ([#1479](#1479)) ([96adc82](96adc82))

### Features

* display purchase details in checkout modal when taxes are enabled ([#1480](#1480)) ([b69f8f3](b69f8f3))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.72.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Jul 3, 2023
# [1.72.0](v1.71.0...v1.72.0) (2023-07-03)

### Bug Fixes

* **donate-block:** allow spaces in button label ([#1479](#1479)) ([96adc82](96adc82))

### Features

* display purchase details in checkout modal when taxes are enabled ([#1480](#1480)) ([b69f8f3](b69f8f3))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.72.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants