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.

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

"Submit for Review" button regression on mobile #10475

Closed
jasmussen opened this issue Oct 10, 2018 · 6 comments · May be fixed by makbarGroup/gutenberg#31
Closed

"Submit for Review" button regression on mobile #10475

jasmussen opened this issue Oct 10, 2018 · 6 comments · May be fixed by makbarGroup/gutenberg#31
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Milestone

Comments

@jasmussen
Copy link
Contributor

jasmussen commented Oct 10, 2018

As discovered in #9495 (comment), the fact that we allow the pre-publish checks to be turned off causes the editor bar to not fit on mobile (notably iPhone 5 size screens in german), where the width of the publish button label can say "Zur Überprüfung einreichen...".

Suggested solution: if a user disables the pre-publish checks, this only works on larger than mobile breakpoints, and on mobile, the pre-publish checks are always enabled. This is similar to how the "Unified Toolbar" function works: this is disabled on mobile because it technically isn't feasible to dock the toolbar to the top on mobile web due to how iphone treats fixed position.

CC: @nosolosw

@jasmussen jasmussen added [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release labels Oct 10, 2018
@jasmussen jasmussen added this to the 4.1 milestone Oct 10, 2018
@gziolo gziolo added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Oct 16, 2018
@gziolo
Copy link
Member

gziolo commented Oct 16, 2018

There is no corresponding PR, moving to 4.2.

@nfmohit
Copy link
Member

nfmohit commented Oct 19, 2018

Submitted #10803 addressing this issue ❤️

@oandregal
Copy link
Member

Hey, I don't understand how the "Pre-publish checks" play any role here. To make sure I follow the problem: is it that the editor bar grows horizontally in mobile beyond the viewport?

At least this happens when the "publish" button text is longer than a few characters, for example: when a contributor's post has been published (so the button shows the "Submit for review"). This screenshot shows how the UI looks like for mobile viewport both when a contributor's post has been published but is waiting for review (pre-publish checks enabled) and when the pre-publish checks are enabled (so the publish buttons shows the the "schedule for review" text).

contributor-submit-for-review

Perhaps we need to work here on preventing the Publish button from growing horizontally?

@jasmussen
Copy link
Contributor Author

@nosolosw

Hey, I don't understand how the "Pre-publish checks" play any role here. To make sure I follow the problem: is it that the editor bar grows horizontally in mobile beyond the viewport?

The way we solved this in the past was to make sure that the button always says "Publish...", which is a short label that fits on mobile.

But that label only makes sense if it opens up the pre-publish checks. If a user disables those pre-publish checks, the top level button shows the direct action.

The solution proposed so far, which I believe is implemented in #10803, is to simply always show the pre-publish checks on mobile. On a small screen with limited real estate, I think it's okay to enforce the pre-publish checks as an extra confirm action, regardless of the users desktop setting.

@oandregal
Copy link
Member

oandregal commented Oct 23, 2018

As a follow-up from a slack convo with Joen: #10803 doesn't fix the issue. The "Submit for review" button will be shown anyway due to this check. Once the user hits the "Submit for review" button in the pre-publish checks modal, the post changes state to isPending so the next time the toggle won't be shown, but the button.

I'm trying to understand what that check is for and the side-effects it may have to change it. Will follow up with a PR to fix this.

@oandregal
Copy link
Member

#10941 refactors the code and bypasses the isPending condition on small mobile viewports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
4 participants