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

Remove wrap text setting in button block #6567

Closed

Conversation

samikeijonen
Copy link
Contributor

@samikeijonen samikeijonen commented May 3, 2018

Description

There is a Wrap text setting in Button block. However it doesn't do anything. It doesn’t add any classes and no visual change in the editor or front-end. See issue #6563.

How has this been tested?

Tested changing other button settings.

Screenshots

button-block-settings

Note from screenshot. There is kind of weird looking "double" border now.

Types of changes

Remove wrap text setting in button block

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@aduth
Copy link
Member

aduth commented May 3, 2018

Previously: #1512

Is #1473 an issue with or without these changes?

@samikeijonen
Copy link
Contributor Author

Hmm I still don't get what it's suppose to do :)

Screenshot:

wrap text toggle with text

  1. Add button.
  2. Align left.
  3. Wrap text to on state.
  4. Add paragraph.

What's suppose to happen? Should button be in it's own line?

@aduth
Copy link
Member

aduth commented May 3, 2018

I believe that's what the option is for, to allow you to choose between floated left (with content wrapped) and left-aligned on its own line.

@ZebulanStanphill
Copy link
Member

@aduth Perhaps this is an indication that that toggle needs to be renamed? I would have guessed that the "Wrap text" option would toggle whether text inside the button would wrap. (Even though that would be a pretty weird thing to dedicate a toggle to.)

@youknowriad
Copy link
Contributor

Unless I'm missing something, it's not doing anything on the frontend, in that case I agree with @samikeijonen we should remove it or correct the frontend behavior.

@samikeijonen
Copy link
Contributor Author

samikeijonen commented May 3, 2018

Perhaps this is an indication that that toggle needs to be renamed? I would have guessed that the "Wrap text" option would toggle whether text inside the button would wrap

Definitely needs more clear wording. I thought it was about text inside the button.

Actually another issue for me is that even if I toggle Wrap text on, update, then refresh the page, and the setting is not saved at all. Maybe that's the real issue here and therefore doesn't do anything for me.

@samikeijonen
Copy link
Contributor Author

This should be fixed in #7110.

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.

None yet

4 participants