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.

Sign up for GitHub

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

Jump to bottom

Deleting an Additional CSS Class attribute leaves an empty className attribute in the HTML comment code #11664

Closed
dianeco opened this issue Nov 9, 2018 · 2 comments · Fixed by #11831
Closed

Deleting an Additional CSS Class attribute leaves an empty className attribute in the HTML comment code #11664

dianeco opened this issue Nov 9, 2018 · 2 comments · Fixed by #11831
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.

Comments

@dianeco
Copy link

dianeco commented Nov 9, 2018

Describe the bug
When you delete an Additional CSS Class, it leaves an empty className attribute in the HTML comment code.

To Reproduce
Steps to reproduce the behavior:

  1. Insert a heading block and type some text
  2. In the Advanced panel, set an Additional CSS class
  3. Empty the Additional CSS class field
  4. Switch to Code Editor.
<!-- wp:heading {"className":""} -->
<h2>Heading</h2>
<!-- /wp:heading -->

Desktop:

  • OS: Windows 10, Mac OS
  • Browser; Chrome, Safari Firefox, Edge

Additional context
Gutenberg 4.2.0
@designsimply designsimply added the Needs Testing Needs further testing to be confirmed. label Nov 11, 2018
@designsimply
Copy link
Member

Tested and confirmed using WordPress 4.9.8 and Gutenberg 4.3 that adding and removing an "Additional CSS Class" to the heading block results in a leftover empty className field in the block's HTML comment. (23s)

empty-classname-in-comment
Seen at http://alittletestblog.com/wp-admin/post.php?post=15720&action=edit running WordPress 4.9.8 and Gutenberg 4.3 using Firefox 63.0.1 on macOS 10.13.6.

@designsimply designsimply added [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement. and removed Needs Testing Needs further testing to be confirmed. labels Nov 12, 2018
@tofumatt tofumatt added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Nov 12, 2018
@gziolo gziolo added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Nov 12, 2018
@gziolo
Copy link
Member

gziolo commented Nov 12, 2018

className is set to an empty string in this place:

https://github.com/WordPress/gutenberg/blob/master/packages/editor/src/hooks/custom-class-name.js#L63-L70

@ninio ninio mentioned this issue Nov 16, 2018
Merged
4 tasks
jorgefilipecosta pushed a commit that referenced this issue Jan 30, 2019
@ninio @jorgefilipecosta
Removed unnecessary className attribute. Fixes #11664 (#11831)
b09c855 
## Description
Added a check for empty string to the `onChange` handler of the "Additional CSS Class field".
If the string is empty the `setAttributes` is called with `{ className: undefined }`
This will resolve #11664


## How has this been tested?
Created a new post and added a heading block.
In the Advanced menu, added a class and switched to the Code Editor

Tested on local WP running on Windows with Chrome 70.0.3538.102

## Types of changes
Bug fix (non-breaking change which fixes an issue)

## Checklist:
- [x] My code is tested.
- [x] My code follows the WordPress code style. <!-- Check code: `npm run lint`, Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/ -->
- [x] My code follows the accessibility standards. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/accessibility-coding-standards/ -->
- [x] My code has proper inline documentation. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/javascript/ -->
daniloercoli added a commit that referenced this issue Jan 30, 2019
@daniloercoli
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
68b353c 
…rnmobile/372-use-RichText-on-Title-block

* 'master' of https://github.com/WordPress/gutenberg: (36 commits)
  Fixes plural messages POT generation. (#13577)
  Typo fix (#13595)
  REST API: Remove oEmbed proxy HTML filtering (#13575)
  Removed unnecessary className attribute. Fixes #11664 (#11831)
  Add changelog for RSS block (#13588)
  Components: Set type=button for TabPanel button elements. (#11944)
  Update util.js (#13582)
  Docs: Add accessbility specific page (#13169)
  Rnmobile/media methods refactor (#13554)
  chore(release): publish
  chore(release): publish
  Plugin: Deprecate gutenberg_get_script_polyfill (#13536)
  Block API: Parse entity only when valid character reference (#13512)
  RichText: List: fix indentation (#13563)
  Plugin: Deprecate window._wpLoadGutenbergEditor (#13547)
  Plugin: Avoid setting generic "Edit Post" title on load (#13552)
  Plugin: Populate demo content by default content filters (#13553)
  RichText: List: Fix getParentIndex (#13562)
  RichText: List: Fix outdent with children (#13559)
  Scripts: Remove npm run build from test-e2e default run (#13420)
  ...
youknowriad pushed a commit that referenced this issue Mar 6, 2019
@ninio @youknowriad
Removed unnecessary className attribute. Fixes #11664 (#11831)
065b26c 
## Description
Added a check for empty string to the `onChange` handler of the "Additional CSS Class field".
If the string is empty the `setAttributes` is called with `{ className: undefined }`
This will resolve #11664


## How has this been tested?
Created a new post and added a heading block.
In the Advanced menu, added a class and switched to the Code Editor

Tested on local WP running on Windows with Chrome 70.0.3538.102

## Types of changes
Bug fix (non-breaking change which fixes an issue)

## Checklist:
- [x] My code is tested.
- [x] My code follows the WordPress code style. <!-- Check code: `npm run lint`, Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/ -->
- [x] My code follows the accessibility standards. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/accessibility-coding-standards/ -->
- [x] My code has proper inline documentation. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/javascript/ -->
youknowriad pushed a commit that referenced this issue Mar 6, 2019
@ninio @youknowriad
Removed unnecessary className attribute. Fixes #11664 (#11831)
213f004 
## Description
Added a check for empty string to the `onChange` handler of the "Additional CSS Class field".
If the string is empty the `setAttributes` is called with `{ className: undefined }`
This will resolve #11664


## How has this been tested?
Created a new post and added a heading block.
In the Advanced menu, added a class and switched to the Code Editor

Tested on local WP running on Windows with Chrome 70.0.3538.102

## Types of changes
Bug fix (non-breaking change which fixes an issue)

## Checklist:
- [x] My code is tested.
- [x] My code follows the WordPress code style. <!-- Check code: `npm run lint`, Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/ -->
- [x] My code follows the accessibility standards. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/accessibility-coding-standards/ -->
- [x] My code has proper inline documentation. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/javascript/ -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Removed unnecessary className attribute. Fixes #11664 ninio/gutenberg
4 participants
@tofumatt @gziolo @designsimply @dianeco