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

Add a check on Marker Popup to be displayed only when block is… #14241

Merged
merged 2 commits into from
Dec 16, 2019
Merged

Add a check on Marker Popup to be displayed only when block is… #14241

merged 2 commits into from
Dec 16, 2019

Conversation

thrijith
Copy link
Contributor

Add a check on Marker Popup to be displayed only when block is selected

Fixes #14057

Changes proposed in this Pull Request:

  • Add a condition to add <AddPoint> component only when the block is selected.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • It updates the Map block, and adds a check on the Marker Popup so that it is only displayed when block is selected.

Testing instructions:

Before
Here the Markdown block is selected, but the popup is still present.
before

After
Popup behaviour when Map block is selected.
after-1
Popup behaviour when Map block is not selected.
after-2

Proposed changelog entry for your changes:

Fix Marker Popup UI issue in Map block.

@thrijith thrijith requested a review from a team December 15, 2019 16:34
@jetpackbot
Copy link

jetpackbot commented Dec 15, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 14, 2020.
Scheduled code freeze: January 7, 2020

Generated by 🚫 dangerJS against 4b0d44f

@jeherve jeherve added [Block] Map [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Bug When a feature is broken and / or not performing as intended labels Dec 16, 2019
@jeherve jeherve added this to the 8.1 milestone Dec 16, 2019
jeherve
jeherve requested changes Dec 16, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This seems to work well. What do you think about setting a constant here and using it, so if we need it later on for something else, it will already be available?

@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 16, 2019
@thrijith
Copy link
Contributor Author

thrijith commented Dec 16, 2019 •
edited
Loading

@jeherve updated usage, initially I did that but went back to direct usage from props as there was no other need for it.

jeherve
jeherve approved these changes Dec 16, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This looks good to me. 👍 @jeffersonrabb Want to take a look at it as well?

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Dec 16, 2019
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello thrijith! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D36796-code before merging this PR. Thank you!

jeffersonrabb
jeffersonrabb approved these changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@jeffersonrabb jeffersonrabb left a comment

Choose a reason for hiding this comment

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

Nice improvement, works well. 🚢

@kraftbj kraftbj changed the title Add a check on Marker Popup to be displayed only when block is selected Add a check on Marker Popup to be displayed only when block is… Dec 16, 2019
@kraftbj kraftbj merged commit ed104a2 into Automattic:master Dec 16, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 16, 2019
@kraftbj
Copy link
Contributor

kraftbj commented Dec 16, 2019

r200790-wpcom

@thrijith thrijith deleted the fix/map-popup-visibility branch December 17, 2019 05:44
jeherve added a commit that referenced this pull request Dec 20, 2019
@jeherve
Changelog: add #14241
c36a339
zinigor added a commit that referenced this pull request Dec 30, 2019
@jeherve @zinigor
Changelog: 8.1 additions (#14166)
fff089c 
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
zinigor added a commit that referenced this pull request Dec 30, 2019
@jeherve @zinigor
Changelog: 8.1 additions (#14166)
d4a7565 
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffersonrabb jeffersonrabb jeffersonrabb approved these changes

@jeherve jeherve jeherve approved these changes

Assignees

@jeherve jeherve

@thrijith thrijith

Labels
[Block] Map Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
8.1
Development

Successfully merging this pull request may close these issues.

Map Block: Marker popup is visible when the block isn't focussed
6 participants
@thrijith @jetpackbot @matticbot @kraftbj @jeherve @jeffersonrabb