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

Try improving mobile controls #4101

Merged
merged 4 commits into from Dec 20, 2017

Conversation

Projects
None yet
3 participants
@jasmussen
Contributor

jasmussen commented Dec 20, 2017

Those controls include the mover and the ellipsis menu. As such, this PR fixes #4084.

What this PR does, is remove all mobile tools from the quick toolbar, and instead shows these tools inline at the bottom of a selected block.

GIF:

mobile tools

Credit goes to @iamthomasbishop for the concept.

As this PR does some surgery to existing components, I'd appreciate a good code review.

If combined with #4081, it will drastically improve the mobile interface.

Try improving mobile controls
Those controls include the mover and the ellipsis menu. As such, this PR fixes #4084.

What this PR does, is remove all mobile tools from the quick toolbar, and instead shows these tools inline at the bottom of a _selected_ block.
@karmatosed

Really like this 👍

@@ -19,40 +19,3 @@
display: inline-flex;
border-left: 1px solid $light-gray-500;
}

This comment has been minimized.

@youknowriad

youknowriad Dec 20, 2017

Contributor

There's a mention of .editor-block-toolbar__mobile-tools in this file, I guess we can remove it right now?

@youknowriad

youknowriad Dec 20, 2017

Contributor

There's a mention of .editor-block-toolbar__mobile-tools in this file, I guess we can remove it right now?

This comment has been minimized.

@jasmussen

jasmussen Dec 20, 2017

Contributor

Fixed this one, and found it another place too, thanks.

@jasmussen

jasmussen Dec 20, 2017

Contributor

Fixed this one, and found it another place too, thanks.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Dec 20, 2017

Contributor

I like how this simplifies the code :)

Contributor

youknowriad commented Dec 20, 2017

I like how this simplifies the code :)

@youknowriad

LGTM 👍 I wonder if it would be better in term of "design" to make it feel like a real toolbar with a border around the buttons (same as the upper toolbar).

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Dec 20, 2017

Contributor

Thanks for the review, and for pushing fixes ❤️ ! I'm going to merge this early because it's going to be good to have the holidays to test this.

I wonder if it would be better in term of "design" to make it feel like a real toolbar with a border around the buttons (same as the upper toolbar).

I'm worried about "toolbaritis", and I like the idea of reusing the pattern of unselected block is a previes, and selected block can show extra controls, same as the Button and Image blocks do. But obviously this is something we'll want to polish as we go.

Contributor

jasmussen commented Dec 20, 2017

Thanks for the review, and for pushing fixes ❤️ ! I'm going to merge this early because it's going to be good to have the holidays to test this.

I wonder if it would be better in term of "design" to make it feel like a real toolbar with a border around the buttons (same as the upper toolbar).

I'm worried about "toolbaritis", and I like the idea of reusing the pattern of unselected block is a previes, and selected block can show extra controls, same as the Button and Image blocks do. But obviously this is something we'll want to polish as we go.

@jasmussen jasmussen merged commit c89e226 into master Dec 20, 2017

3 checks passed

codecov/project 39.14% (+0.19%) compared to 64ac510
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jasmussen jasmussen deleted the try/improved-mobile-tools branch Dec 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment