Skip to content

Widgets - EU Cookie Law: fix issue with custom URL choice and selective refresh #9027

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
Merged
zinigor merged 1 commit into from
Mar 27, 2018
Merged

Widgets - EU Cookie Law: fix issue with custom URL choice and selective refresh #9027

zinigor merged 1 commit into from
Mar 27, 2018

Conversation

@eliorivero
Copy link
Contributor

@eliorivero eliorivero commented Mar 9, 2018 •
edited
Loading

Fixes #8998

Changes proposed in this Pull Request:

  • fix issue where custom URL choice wasn't preserved in Customizer
  • fix issue where widget didn't selectively refresh in Customizer. While the widget declared support, it didn't reinitialized itself after it was selectively refreshed.

Testing instructions:

  • add a EU Cookie Law Banner widget and make sure you can choose a Custom URL for the policy
  • make sure selective refresh works fine

Proposed changelog entry for your changes:

@eliorivero
Widgets - EU Cookie Law: fix issue where custom URL choice wasn't pre…
7c74ec7 
…served in Customizer. Fix issue where widget didn't selectively refresh in Customizer.
@eliorivero eliorivero added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets [Status] Needs Review This PR is ready for review. labels Mar 9, 2018
@eliorivero eliorivero added this to the 6.0 milestone Mar 9, 2018
@eliorivero eliorivero requested a review from Copons March 9, 2018 22:10
@eliorivero eliorivero requested a review from a team as a code owner March 9, 2018 22:10
@Copons
Copy link
Contributor

Copons commented Mar 12, 2018

Code LGTM and fixes the issue.

I'd just like to point out the reasoning behind the previous behaviour: IIRC it was to prevent folks to submit a widget with an empty policy URL.
In other words, you can check the Custom radio only if the URL field is not empty.
Which is totally counter-intuitive, and the widget is already guarded against the empty URL by defaulting it to the default URL anyway. 🙂

Copons
Copons approved these changes Mar 14, 2018
View reviewed changes
Copy link
Contributor

@Copons Copons left a comment

Choose a reason for hiding this comment

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

:shipit:

@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 Mar 27, 2018
@zinigor zinigor merged commit 4eef310 into master Mar 27, 2018
@zinigor zinigor deleted the fix/cookie-law-custom-policy-url branch March 27, 2018 21:07
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 27, 2018
dereksmart added a commit that referenced this pull request Mar 27, 2018
@dereksmart
add #9027
36958e3
dereksmart pushed a commit that referenced this pull request Mar 27, 2018
@oskosk @dereksmart
Changelog 6.0: running changelog (#9034)
f54fc7b 
* Changelog 6.0: create base for changelog.

* Add #8938 to changelog

* Add #8962 to changelog

* Add #8974 to changelog

* Add #8975 to changelog

* Add #8978 to changelog

* Add #8867 to changelog

* Add #8937 to changelog

* Add #8961 to changelog

* Add #8855 to changelog

* Add #8944 to changelog

* Add #8973 to changelog

* Add #8977 to changelog

* Add #8979 to changelog

* Add #8980 to changelog

* Add #8982 to changelog

* Add #8983 to changelog

* Add #8984 to changelog

* Add #8986 to changelog

* Add #9005 to changelog

* Add #9010 to changelog

* Add #9012 to changelog

* Add #9021 to changelog

* Add #9022 to changelog

* Add #9056 to changelog

* Add #9061 to changelog

* Add #9079 to changelog

* Add #9080 to changelog

* Add #9088 to changelog

* Add #9096 to changelog

* Add #9097 to changelog

* Add #9100 to changelog

* Add #9107 to changelog

* Add #8969 to changelog

* Add #8993 to changelog

* Add #9003 to changelog

* Add #9031 to changelog

* Add #8945 to changelog

* Add #9052 to changelog

* Add #9058 to changelog

* Add #9066 to changelog

* Add #9076 to changelog

* Add #9053 to changelog

* Add #9108 to changelog

* Add #9135 to changelog

* Add #9148 to changelog

* Add #9125 to changelog

* Add #9137 to changelog

* Added testing instructions for 6.0.

* Added IS testing instructions, huge props to @tiagonoronha.

* Added #8498 to changelog.

* Added #8954 to changelog.

* Added #8985 to changelog.

* add #9027

* add #9112 to changelog

* add #9136 to changelog

* add #9102 to changelog

* add #9093 to changelog

* add #9062 to changelog

* add #9172 to changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Copons Copons Copons approved these changes

Assignees

No one assigned

Labels

[Feature] Extra Sidebar Widgets [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Milestone

6.0

Development

Successfully merging this pull request may close these issues.

EU Cookie Law Banner Widget: Policy URL switches from Custom to Default

6 participants

@eliorivero @Copons @zinigor @dereksmart @matticbot