Skip to content

Widget Visibility: fix widget minor conditions dropdown entities#8425

Merged
oskosk merged 1 commit intoAutomattic:masterfrom
zigasancin:fix/widget-minor-conditions-dropdown-entities
Feb 27, 2018
Merged

Widget Visibility: fix widget minor conditions dropdown entities#8425
oskosk merged 1 commit intoAutomattic:masterfrom
zigasancin:fix/widget-minor-conditions-dropdown-entities

Conversation

@zigasancin
Copy link
Copy Markdown
Contributor

@zigasancin zigasancin commented Dec 27, 2017

The option items of the minor conditions dropdown in the widget visibility settings are being correctly handled with esc_attr in the backend. However, some of the category names may contain entities (e.g. ampersands), so those option items must be decoded in the frontend to make the text human-readable in these cases.

Changes proposed in this Pull Request:

  • Adding a decodeEntities function in widget-conditions.js to handle entity decoding for the minor conditions dropdown.

Testing instructions:

  • Check the minor conditions dropdown with a category name that contains an entity (e.g. ampersand) in the widget visibility settings with and without this patch. Don't forget to bump the version number of the javascript file while it is enqueued.

@zigasancin zigasancin requested a review from a team as a code owner December 27, 2017 15:54
@jeherve jeherve added [Feature] Widget Visibility [Status] Needs Review This PR is ready for review. Bug When a feature is broken and / or not performing as intended labels Dec 29, 2017
@zigasancin zigasancin requested a review from jeherve as a code owner January 4, 2018 19:21
@zigasancin
Copy link
Copy Markdown
Contributor Author

zigasancin commented Jan 4, 2018

Oh my, merged the wrong branch... Sorry.

@oskosk
Copy link
Copy Markdown
Contributor

oskosk commented Jan 4, 2018

here's how to rebase your branches:

https://github.com/Automattic/jetpack/blob/master/docs/git-workflow.md#keeping-your-pr-up-to-date-as-an-external-contributor

@zigasancin
Copy link
Copy Markdown
Contributor Author

zigasancin commented Jan 5, 2018 •
edited
Loading

Ooops, forgot about this part of the documentation. Thanks!
But I have a problem when running rebase - it returns:

fatal: Needed a single revision
invalid upstream jetpack/master

Maybe I should try with git rebase -i --root?

@oskosk
Copy link
Copy Markdown
Contributor

oskosk commented Jan 5, 2018

If you followed those steps, can you try this instead...

$ git fetch jetpack
$ git rebase jetpack/master

@zigasancin
Widget Visibility: fix widget minor conditions dropdown entities
bd140de 
The option items of the minor conditions dropdown in the widget
visibility settings are being correctly handled with esc_attr in the
backend. However, some of the category names may contain entities (e.g.
ampersands), so those option items must be decoded in the frontend to
make the text human-readable in these cases.
dereksmart
dereksmart approved these changes Feb 20, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

LGTM -- works as expected

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 20, 2018
@oskosk oskosk added this to the 5.9 milestone Feb 23, 2018
oskosk
oskosk approved these changes Feb 27, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM!

oskosk added a commit that referenced this pull request Feb 27, 2018
@oskosk
Add changelog entry for #8425
7d9de8b
@oskosk oskosk merged commit a75de0c into Automattic:master Feb 27, 2018
oskosk added a commit that referenced this pull request Feb 27, 2018
@oskosk
Changelog 5.9: running changelog (#8819)
1174ef5 
* update changelog.txt

* Update readme.txt with scaffolding for 5.9 changelog and release draft shortlink

* Add changelog entry for #8243

* Add changelog entry for #8296

* Add changelog entry for #8367

* Add changelog entry for #8686

* Add changelog entry for #8707

* Add changelog entry for #8709 and #8714

* Add changelog entry for #8729

* Add changelog entry for #8777

* Add changelog entry for #8780

* Add changelog entry for #8786

* Add changelog entry for #8787

* Add changelog entry for #8801 #8805 #8832 #8865 and #8804

* Add changelog entry for #8817

* Add changelog entry for #8822

* Add changelog entry for #8823

* Add changelog entry for #8829

* Add changelog entry for #8834

* move some items to major enhancements

* Add changelog entry for #8836

* Add changelog entry for #8839

* Add changelog entry for #8861

* Add changelog entry for #8862

* Add changelog entry for #8863

* Add changelog entry for #8866

* Add changelog entry for #8870

* Add changelog entry for #8874

* Add changelog entry for #8875

* Add changelog entry for #8881

* Add changelog entry for #8890

* Add changelog entry for #8911

* Add changelog entry for #8927

* Add changelog entry for #8931

* Add changelog entry for #8933

* Add changelog entry for #8930

* fix wording

* typo

* minor fixes

* replace partner scripts for Jetpack Start in changelog entry

* Update to-test.md

* Update to-test.md

* minor style fixes to to-test.md

* minor style fixes to to-test.md

* minor fixes on to-test.md

* Add changelog entry for #8868

* Add changelog entry for #8844

* Add changelog entry for #8664

* Add changelog entry for #8935

* Add changelog entry for #8425

* Add changelog entry for #8625
@zigasancin zigasancin deleted the fix/widget-minor-conditions-dropdown-entities branch March 1, 2018 11:21
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dereksmart dereksmart dereksmart approved these changes

@jeherve jeherve Awaiting requested review from jeherve

+1 more reviewer

@oskosk oskosk oskosk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Widget Visibility Touches WP.com Files

Projects

None yet

Milestone

5.9

Development

Successfully merging this pull request may close these issues.

6 participants

@zigasancin @oskosk @dereksmart @kraftbj @jeherve @matticbot