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

Fix issues with sticky quick toolbar positioning #2834

Merged
merged 3 commits into from Sep 29, 2017
Merged

Fix issues with sticky quick toolbar positioning #2834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f7bc31
Fix sticky position issues.
Sep 29, 2017
ad06ecb
Improve sticky toolbar positioning, classic text
Sep 29, 2017
55129ad
Fix issue where the kitchen sink pushed down content.
Sep 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion blocks/library/freeform/editor.scss
View file
Open in desktop
Expand Up @@ -92,7 +92,7 @@
position: sticky;

@include break-medium() {
top: $header-height + $admin-bar-height + $item-spacing;
top: $item-spacing;
}
}

Expand Down Expand Up @@ -146,6 +146,10 @@
display: block;
}

.freeform-toolbar.has-advanced-toolbar {
margin-top: -85px; /* Pull upwards the classic text toolbar when enabled */
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it always 85 pixels? I guess the advanced toolbar could have several rows?

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried installing TinyMCE advanced, and I see two issues, when activating the plugin, I see the "menu" toolbar pushing the block's toolbar a bit. And when adding an extra row in the settings, It's not showing up as expected too.

screen shot 2017-09-29 at 09 55 11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solid observation, and good point. I don't know what we can do to fix this — I know it seems hacky, but unless we have some way to calculate the height of the fully expanded toolbar, I can't think of a way to "work perfectly" in every toolbar situation. Especially since responsiveness is going to introduce still more issues.

That said, the above seems to work fairly well considering you have three toolbars. All content is editable, all content is visible, and there is room for the toolbars. Since one key philosophy is that unselected block is preview and selected block/edit mode can show more options, although not optimal this doesn't fall outside of the principles.

Since Gutenberg comes with up to two toolbars by default, I feel like this is a decent compromise, especially since three stacked toolbars is an editor design I don't think is very good in the first place, but there for back compat purposes.

What do you think, @karmatosed?

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, good points, I'm ok with this tradeoff

}

.freeform-toolbar div.mce-btn-group {
padding: 0;
margin: 0;
Expand Down
2 changes: 1 addition & 1 deletion editor/modes/visual-editor/style.scss
View file
Open in desktop
Expand Up @@ -340,7 +340,7 @@
}

@include break-medium() {
top: $header-height + $admin-bar-height + $item-spacing;
top: $item-spacing;
}

&.is-appearing-active {
Expand Down