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

Layout: Improve layout paneling for short viewports #2909

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Oct 6, 2017

Related: #2800

This pull request seeks to resolve styling issues observed related to paneling introduced in #2800 , specifically in short viewports or dashboards where many sidebar navigation items exist (I first noticed the issue when using Gutenberg on a WordCamp site). The changes here try an alternative approach for occupying the full height of the screen in more contexts. Previously, the specific height calc style would not account for cases where the sidebar has a scrollbar.

Testing instructions:

Repeat testing instructions from #2800, particularly with varying viewport widths and heights and sidebars expanded and collapsed.

@aduth aduth added the General Interface Parts of the UI which don't fall neatly under other labels. label Oct 6, 2017
@gziolo
Copy link
Member

gziolo commented Oct 9, 2017

Testing instructions :

Copied from #2800.

  1. Navigate to Gutenberg > Demo or Gutenberg > New Post
  2. Note that an "Extended Settings" toggle heading is shown at the bottom of the page
  3. Toggle the Extended Settings panel
  4. Note that the panel expands upward, and that it is still possible to reach all content of the post while the panel is expanded

@aduth aduth force-pushed the fix/editor-canvas-short-screen branch 2 times, most recently from ac3704f to 3a03480 Compare October 27, 2017 12:39
@aduth aduth requested a review from jasmussen October 27, 2017 12:40
@aduth
Copy link
Member Author

aduth commented Oct 27, 2017

Rebased to resolve conflicts with the stacked toolbar introduced in #2998.

Since it wasn't well illustrated in my original comment, here is a before and after for when scrolling to the bottom while the viewport is shorter than the menu:

Before After
Before After

@jasmussen
Copy link
Contributor

Visually this looks good to me, it seems to behave as it should responsively! Nice work.

I'm seeing an issue with what's probably pasting. I have this textexpansion I use for testing lots of text:

Lorem ipsum dolor sit amet, ferri vidisse nam eu, ad nec copiosae mnesarchum vituperatoribus. Te brute dicunt sea, ut vis omnium menandri, ut sumo aliquam has. Eum aperiam interpretaris at, sea et recusabo expetenda, omnis tibique mea no. Pri suas partem ea, ius sonet numquam offendit cu, ad simul admodum pri. Eum cu unum choro albucius.

Eu integre accusata prodesset est, sed te impetus gubergren conceptam, ex sed wisi nostrum ocurreret. Esse velit omittantur ius te, alii dissentias ei vis. At sed unum veritus fabellas. Te volutpat appellantur duo. Ad natum fuisset intellegebat eam, causae invidunt usu id, et vis impetus appetere.

Ea veniam homero eam. Ex inimicus molestiae cum, debet scaevola at eos. Vis assum veritus ut, has ea nostrud accusata, offendit appareat comprehensam ea pro. Ad quo quem veritus appellantur, te est quas phaedrum, eum alia habeo ad. Ei est erroribus imperdiet, omnis dicam propriae sed no. His vitae oratio fierent ne, cu duo tota eligendi, electram rationibus in qui.

Est quis reque cetero ad. Sea id autem nominavi deseruisse. Veniam qualisque definitionem pri id, ea autem feugiat delenit ius, mei at lorem affert accumsan. Dicat eruditi cu est, te pro dicant pericula conclusionemque, ei vim detracto euripidis intellegam. Eius postea volumus mei ad.

Prima ridens denique his te, ferri illum volumus an his. Eu vel dicat homero qualisque, vitae regione deserunt vis ei. Graeci incorrupte liberavisse no mea, saepe voluptaria usu ex, vis dicant euismod id. At dolor reprimique eos, quo altera detraxit moderatius id. Quo iudico utinam eu, ad alia munere mel.

When this expands in master, it works fine. When I expand that in this branch, I get the following error:

screen shot 2017-10-30 at 10 03 41

Is that a node caching ghost or an issue with this branch?

@aduth aduth force-pushed the fix/editor-canvas-short-screen branch from 3a03480 to 6f6ee6e Compare October 30, 2017 18:57
@aduth
Copy link
Member Author

aduth commented Oct 30, 2017

Is that a node caching ghost or an issue with this branch?

Since there's only styling changes here, I don't expect I could introduce an unhandled error from these revisions, or one would hope 😄

Instead, I think it's just the time that this branch was merged there was an issue in master, probably introduced in #3171 and later resolved in #3205.

I rebased and force pushed. If you delete your local copy of the branch (git branch -D fix/editor-canvas-short-screen) and check it out again, it should work okay now.

@jasmussen
Copy link
Contributor

Can confirm, those JS errors are gone. 👍 👍 nice work!

@aduth aduth merged commit b42dbed into master Oct 31, 2017
@aduth aduth deleted the fix/editor-canvas-short-screen branch October 31, 2017 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants