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

Calypsoify: Remove style that accidentally hides "Code editor" option #17264

Merged
merged 1 commit into from
Sep 28, 2020
Merged

Calypsoify: Remove style that accidentally hides "Code editor" option #17264

merged 1 commit into from
Sep 28, 2020

Conversation

WunderBart
Copy link
Member

@WunderBart WunderBart commented Sep 25, 2020 •
edited
Loading

Gutenberg hides the "View" menu for small screens now. It's is causing our selector to miss the target and hide the "Code editor" item instead. Thankfully, since what we want is handled natively now we can safely remove our patch.

Changes proposed in this Pull Request:

Bugfix

Additional context:

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

  • Open iframed editor in mobile view (width < 782px)
  • Open the Options menu (three vertical dots in the top-right corner)
  • Confirm that the Code editor item is available
Before After
Screenshot 2020-09-25 at 15 35 05 Screenshot 2020-09-25 at 15 35 22

Proposed changelog entry for your changes:

  • Calypsoify: Fix a bug that's making "Code editor" unreachable on mobile devices

@WunderBart
Remove style that accidentally hides "Code editor" option
573effd 
Gutenberg hides the "View" menu for small screens now: WordPress/gutenberg@f4300ff#diff-84d6cf5cfecefd4e711bd9d50e8fa5f6R14-R18. It's is causing our selector to miss the target and hide the "Code editor" item instead. Thankfully, since what we want is handled natively now we can safely remove our patch.
@WunderBart WunderBart added the [Type] Bug When a feature is broken and / or not performing as intended label Sep 25, 2020
@WunderBart WunderBart requested review from mmtr and Copons September 25, 2020 13:38
@WunderBart WunderBart self-assigned this Sep 25, 2020
@jetpackbot
Copy link

Warnings
⚠️

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

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17264

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 573effd

@WunderBart WunderBart requested a review from a team September 25, 2020 13:42
@WunderBart WunderBart added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Sep 25, 2020
@sirreal
Copy link
Member

sirreal commented Sep 25, 2020

I can confirm this bug happens on a clean Jetpack site by opening the post editor with/without calypsoify=1 query param:

Without:

Screen Shot 2020-09-25 at 16 17 59

With:

Screen Shot 2020-09-25 at 16 17 43

@sirreal
Copy link
Member

sirreal commented Sep 25, 2020

I prepped a diff for this: D50195-code

Shouldn't this diff have been generated automatically?

sirreal
sirreal approved these changes Sep 25, 2020
View reviewed changes
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Works well in my testing (via D50195-code). Thanks for tracking this one down!

@sirreal
Copy link
Member

sirreal commented Sep 25, 2020

From D50195-code

Before:

Screen Shot 2020-09-25 at 16 28 03

After:

Screen Shot 2020-09-25 at 16 27 28

@sirreal sirreal requested a review from a team September 25, 2020 15:50
@jeherve jeherve added this to the 9.0 milestone Sep 28, 2020
@jeherve
Copy link
Member

jeherve commented Sep 28, 2020

Shouldn't this diff have been generated automatically?

Unfortunately no, Calypsoify is a bit different in both platforms.

@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 fellow Jetpack developers. Label will be renamed soon. labels Sep 28, 2020
jeherve
jeherve approved these changes Sep 28, 2020
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.

Tests well for me. 🚢

@jeherve
Copy link
Member

jeherve commented Sep 28, 2020

Merging this now, I'll let you handle the wpcom counterpart.

@jeherve jeherve merged commit bdb0fb0 into master Sep 28, 2020
@jeherve jeherve deleted the fix/unreachable-code-editor branch September 28, 2020 16:25
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 28, 2020
jeherve added a commit that referenced this pull request Sep 29, 2020
@jeherve
Changelog: add #17264
be2480e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sirreal sirreal sirreal approved these changes

@jeherve jeherve jeherve approved these changes

@mmtr mmtr Awaiting requested review from mmtr

@Copons Copons Awaiting requested review from Copons

Assignees

@WunderBart WunderBart

Labels
[Feature] Calypsoify [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@WunderBart @jetpackbot @sirreal @jeherve @matticbot