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

Change html height to auto when opening carousel #12268

Merged
merged 1 commit into from
May 9, 2019
Merged

Change html height to auto when opening carousel #12268

merged 1 commit into from
May 9, 2019

Conversation

fabwu
Copy link
Contributor

@fabwu fabwu commented May 5, 2019

Implements the fix proposed in this commment.

Fixes #1125

Testing instructions:

It's a bit hard to test as you need a theme which has this issue. I tested it with the wpVoyager theme and the standard wordpress theme and everything worked as expected.

Proposed changelog entry for your changes:

  • Fix closing carousel view brings you back to top on certain themes

@fabwu fabwu requested a review from a team as a code owner May 5, 2019 20:57
@jetpackbot
Copy link

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against d0139c1

@jeherve jeherve added [Pri] Low [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Type] Bug When a feature is broken and / or not performing as intended [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. labels May 6, 2019
@jeherve jeherve added this to the 7.4 milestone May 6, 2019
jeherve
jeherve approved these changes May 9, 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 works well in my tests. 👍

@matticbot
Copy link
Contributor

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

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels May 9, 2019
@jeherve jeherve merged commit e76dbc2 into Automattic:master May 9, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 9, 2019
jeherve added a commit that referenced this pull request May 17, 2019
@jeherve
Changelog: add #12268
d24f952
jeherve added a commit that referenced this pull request May 23, 2019
@jeherve
Changelog: add #12268
e46fd4a
jeherve added a commit that referenced this pull request May 27, 2019
@kraftbj @jeherve
7.4 Changelog (#12304)
0fb4436 
* Kick off the changelog

* Add 7.3.1

* Update date and post link

* changelog: add #12219

* changelog: add #12170

* changelog: add #12184

* Changelog: add #12268

* Changelog: add #12081

* Changelog: add #12323

* Changelog: add #12204

* Changelog: add #12269

* Changelog: add #12332

* changelog: add #12339

* changelog: add #12209

* Changelog: add #12319

* Changelog: add #12357

* Changelog: add #12124

* Changelog: add #12373

* Changelog: add #12252

* Changelog: add #12383

* Changelog: add #12372

* changelog: add #12337

* Changelog: add #12290

* Changelog: add #12301

* Changelog: add #12061

* Testing list: add instructions for #12061

* Changelog: add #12393

* Update minimum supported version

See #12287

* Changelog: add #12406

* Testing list: add #12406

* Changelog: add #12277

* Changelog: add #12412

* Changelog: add #11318

* Changelog: add #12328

* Changelog: add #12425

* Changelog: add #12380

* Changelog: add #12428

* Changelog: add #12414

* Changelog: add #12395

* Changelog & Testing list: add #12416, #12417, #12418, and #12348

* changelog: add #12379

* Changelog: add #12341

* changelog: add #12444

* Changelog: add #12434

* Changelog: add #12454

* Changelog: add #12460

* Changelog: add #12463

* Changelog: add #12457

* Changelog / testing list: add #10333

* Changelog: add #12467


Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

Assignees
No one assigned
Labels
[Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Pri] Low Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

Carousel: closing carousel view brings you back to the top of the page
4 participants
@fabwu @jetpackbot @matticbot @jeherve