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

Gutenberg/inject css on both page visible and page started and show correct title in web view #13122

Merged
merged 7 commits into from
Oct 14, 2020
Merged

Gutenberg/inject css on both page visible and page started and show correct title in web view #13122

merged 7 commits into from
Oct 14, 2020

Conversation

marecar3
Copy link
Contributor

@marecar3 marecar3 commented Oct 13, 2020 •
edited
Loading

Gutenberg PR: WordPress/gutenberg#26071
Gutenberg mobile PR: wordpress-mobile/gutenberg-mobile#2715

To test:

  1. On the web, create a post with a classic block on a Jetpack site (we should use jurassic ninja to test the classic block on a jetpack site).
  2. Save or publish the post.
  3. In the app, open the post from the previous step in the block editor.
  4. Tap on the classic block.
  5. Tap on the classic block again.
  6. Select the option to "Edit using the web editor."
  7. Observe that classic block is presented and all HTML elements (navigation bar and other) are hidden.
  8. Check that WebView has a good block title (e.g. 'Edit Classic block')

Screenshot:

device-2020-10-13-143231

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@marecar3 marecar3 requested a review from guarani October 13, 2020 18:20
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 13, 2020 •
edited
Loading

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 13, 2020 •
edited
Loading

Warnings
⚠️ PR is missing at least one label.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

This was referenced Oct 13, 2020
Merged
Merged
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 13, 2020 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

cameronvoell
cameronvoell approved these changes Oct 13, 2020
View reviewed changes
Copy link
Contributor

@cameronvoell cameronvoell left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested on a jetpack enabled self-hosted site (jurassic.ninja),

  1. logged in to the app through wordpress.com id
  2. Went to an article with one classic block and one other gutenberg block
  3. Clicked edit using web editor on the unsupported classic block
  4. Clicked login using wordpress.com when the UBE opened
  5. Was able to edit the classic block successfully, without any extra UI showing in the UBE.

guarani
guarani reviewed Oct 13, 2020
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Sharing here that the hamburger menu bar still appeared 2/5 times in tests:

gif showing hamburger menu appearing intermittently in tests

@marecar3
Copy link
Contributor Author

Sharing here that the hamburger menu bar still appeared 2/5 times in tests:

gif showing hamburger menu appearing intermittently in tests

Hey 👋 @guarani

I think that I have fixed all the issues. Let me know if everything is working well on your side. Thanks!

@marecar3
Copy link
Contributor Author

I have tested the PR with:

Pixel 3, Android 11
Nexus 5x, Android 8.1

and it's working good.

@guarani let me know how it's working on your device? Thanks.

@guarani guarani self-requested a review October 14, 2020 15:40
guarani
guarani approved these changes Oct 14, 2020
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Tested and working great 👍 Thanks @marecar3!

@marecar3 marecar3 merged commit 44128e7 into gutenberg/integrate_release_1.38.1 Oct 14, 2020
@marecar3 marecar3 deleted the gutenberg/inject_css_on_both_page_visible_and_page_started_and_show_correct_title_in_web_view branch October 14, 2020 17:23
@guarani guarani mentioned this pull request Oct 15, 2020
Merged
5 tasks
ceyhun pushed a commit that referenced this pull request Oct 15, 2020
@marecar3 @ceyhun
Gutenberg/inject css on both page visible and page started and show c…
249a1eb 
…orrect title in web view (#13122)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guarani guarani guarani approved these changes

@cameronvoell cameronvoell cameronvoell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marecar3 @cameronvoell @guarani