WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 11 months ago

Last modified 9 months ago

#28328 closed task (blessed) (fixed)

Focus on editing, while editing

Reported by: markjaquith Owned by:
Milestone: 4.0 Priority: high
Severity: normal Version: 4.0
Component: Editor Keywords:
Focuses: ui, accessibility, javascript, administration Cc:

Description

The post editor feels like it has been relegated to a box of medium importance on the edit/compose screen. The height of the screen isn't used to full advantage, and editing is a frustrating scrolling-box-within-a-scrolling-box situation with a lot of distractions. I want to move editing closer to the distraction-free editing experience, to better let you focus on your content when you're actually editing it.

Ideas:

  • Let the editor take up the full height of the content. Scroll it with the page. Only one scroll.
  • Pin the header/footer editing toolbars to top/bottom when they would otherwise be scrolled out of view.
  • Fade out toolbar, menu, and other meta boxes

I did a proof-of-concept toolbar-pinning "content scrolls with page" example here: http://codepen.io/markjaquith/pen/uCtGJ

@avryl is working on this as a plugin: https://github.com/avryl/focus

Issues:

  • How to access save/preview in this state?
  • How to access media in this state?
  • How should it transition? When should it transition back? Animation? Fading? hoverIntent(), etc?

Attachments (35)

28328.patch (46.0 KB) - added by iseulde 13 months ago.
28328.2.patch (45.0 KB) - added by iseulde 13 months ago.
28328.3.patch (52.0 KB) - added by iseulde 13 months ago.
28328.4.patch (44.9 KB) - added by iseulde 13 months ago.
28328.5.patch (45.7 KB) - added by iseulde 13 months ago.
28328.6.patch (59.4 KB) - added by iseulde 13 months ago.
28328.7.patch (34.0 KB) - added by azaozz 13 months ago.
28328.8.patch (33.2 KB) - added by azaozz 13 months ago.
28328.9.patch (32.2 KB) - added by iseulde 13 months ago.
28328.10.patch (1.4 KB) - added by iseulde 13 months ago.
28328.11.patch (1.4 KB) - added by iseulde 13 months ago.
28328.12.patch (1.9 KB) - added by iseulde 13 months ago.
28328.13.patch (2.4 KB) - added by iseulde 13 months ago.
28328.14.patch (1.1 KB) - added by afercia 13 months ago.
editor-expand-and-DFW.png (402.3 KB) - added by afercia 13 months ago.
28328.15.patch (2.6 KB) - added by iseulde 13 months ago.
28328.16.patch (4.8 KB) - added by iseulde 13 months ago.
28328.17.patch (2.6 KB) - added by iseulde 13 months ago.
28328.18.patch (2.8 KB) - added by iseulde 13 months ago.
start-of-document.patch (1.0 KB) - added by afercia 13 months ago.
28328.19.patch (8.2 KB) - added by iseulde 13 months ago.
28328.20.patch (3.9 KB) - added by stephdau 12 months ago.
Attempting to address comment:54: Implementing do-not-pin and defaulting to a minimum editor size based on the current viewport, makes some number less "magical" by making them variables devs can tweak. See
28328.21.patch (5.3 KB) - added by stephdau 12 months ago.
Improves upon attachment:28328.20.patch by alo experimenting with sidebar (meta boxes) pinning.
28328.22.patch (5.2 KB) - added by stephdau 12 months ago.
This improves the sidebar pinning 1st implemented in attachment:28328.21.patch, for the new post screen. See next comment for gotchas.
28328.23.patch (5.4 KB) - added by stephdau 12 months ago.
Improves on attachment:28328.22.patch by not proceeding with bottom pinning when the side bar is taller than the left side column.
28328.24.patch (6.1 KB) - added by azaozz 12 months ago.
28328.25.patch (6.5 KB) - added by azaozz 12 months ago.
28328.26.patch (7.1 KB) - added by azaozz 12 months ago.
Screen Shot 2014-08-14 at 11.01.56.png (337.0 KB) - added by stephdau 12 months ago.
28328.27.patch (7.1 KB) - added by stephdau 12 months ago.
same as Andrew's attachment:28328.26.patch, but with fixed sidebar positioning when reaching the end of the document.
28328.28.patch (9.9 KB) - added by azaozz 12 months ago.
28328.29.patch (9.8 KB) - added by azaozz 12 months ago.
28328.30.patch (10.5 KB) - added by azaozz 12 months ago.
39_unpinned.png (73.4 KB) - added by DrewAPicture 12 months ago.
40_pinned.png (96.5 KB) - added by DrewAPicture 12 months ago.

Download all attachments as: .zip

Change History (127)

comment:1 @ircbot15 months ago

This ticket was mentioned in IRC in #wordpress-dev by MarkJaquith. View the logs.

comment:2 @SergeyBiryukov14 months ago

  • Component changed from General to Editor

comment:3 @ircbot14 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:4 follow-up: @sonjanyc13 months ago

Just installed the plugin and tested it. Looks super sleek! Love it!!

I agree the scroll inside the editor has always been very annoying and expanding the editor depending with content improves the User Experience extremely!

Very sleek how the WYSIWYG is sticky to top and word count on bottom, when scrolling. And also how it fades out if you pass the editor! Big props to @avryl

Just talked to @avryl (at #wcsea contributor day) and we feel that it would make most sense to move the "Add Media" button into the WYSIWYG bar (possibly permanent not just adding it on scroll) so it is accessible on scroll as well.

We are also exploring to move the "Visual / Text" toggle into the WYSIWYG bar as well.

comment:5 @iseulde13 months ago

The visual/text toggle could look like it currently looks in my GSoC plugin. (Only the right part is relevant.)

http://make.wordpress.org/core/files/2014/05/Screen-Shot-2014-05-30-at-13.41.37.png

comment:6 @azaozz13 months ago

There is already an editorCommand, shortcut (Alt+Shift+M) and menu item in the MCE menu (when shown) for the media modal. Adding a button would be couple lines of code :)

comment:7 in reply to: ↑ 4 ; follow-up: @pbearne13 months ago

Replying to sonjanyc:

Just talked to @avryl (at #wcsea contributor day) and we feel that it would make most sense to move the "Add Media" button into the WYSIWYG bar (possibly permanent not just adding it on scroll) so it is accessible on scroll as well.

We need to be careful moving the media button down as a number of plug-ins add other buttons to the media button row

comment:8 in reply to: ↑ 7 ; follow-up: @iseulde13 months ago

Replying to pbearne:

We need to be careful moving the media button down as a number of plug-ins add other buttons to the media button row

That's no problem. Plugins can still add buttons there, even if there is nothing on that line by default.

comment:9 in reply to: ↑ 8 @pbearne13 months ago

Replying to avryl:

Replying to pbearne:

We need to be careful moving the media button down as a number of plug-ins add other buttons to the media button row

That's no problem. Plugins can still add buttons there, even if there is nothing on that line by default.

if we move the media button then this location will be come a second class option to put the the button so developers will move their buttons to the toolbar next to the new media button and this will cause space problems

When I was chatting about this plug-in/feature on IRC I made the suggestion that we could add an action to the back(up) scroll to slide the toolbar down to expose the media buttons and slide then back up on the down scroll.

adding a hover action 3px border to the top of the editor button to cause the button to slide down would be good as well

I like this idea because it keeps the media bar a first class location to add button and feel it feel natural to use

I wonder if we can code this to see how/if it works as part of UX testing we are seeing similar UX in mobile browsers with their toolbar's and this will be a smaller UI change

I hope this makes sense

Paul

@iseulde13 months ago

comment:10 @iseulde13 months ago

The patch pins the media button and visual/text tabs as well. Unsure if that's better.

Please question everything in the patch.

comment:11 @iseulde13 months ago

  • Keywords has-patch added

@iseulde13 months ago

comment:12 @iseulde13 months ago

  • @helen suggested adding the publish save button in the bottom toolbar, maybe as a split button with options for date and time, visibility, status etc.
  • What should happen with the TinyMCE statusbar (where you can see the "path" to select dom nodes)?

comment:13 @iseulde13 months ago

  • This should also be disabled for small screen devices.

comment:14 @ircbot13 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

@iseulde13 months ago

comment:16 @helen13 months ago

  • Type changed from enhancement to task (blessed)

@iseulde13 months ago

@iseulde13 months ago

@iseulde13 months ago

comment:17 @iseulde13 months ago

.6 will let the toolbar scroll up when reaching the bottom of the editor. :)

@azaozz13 months ago

comment:18 @ircbot13 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

@azaozz13 months ago

@iseulde13 months ago

comment:19 follow-up: @azaozz13 months ago

In 29049:

Enhance the editor on the Add/Edit Post screens, first run. Props avril, see #28328.

Last edited 13 months ago by azaozz (previous) (diff)

comment:20 @azaozz13 months ago

[29049] is props avryl, sorry for the typo.

comment:21 follow-up: @iseulde13 months ago

We know there are still quite a few issues for smaller screens. We also still need to disable this for mobile devices.

comment:22 @azaozz13 months ago

In 29050:

Fix whitespace, quotes and double patch content in wpautoresize plugin, see #28328

@iseulde13 months ago

@iseulde13 months ago

@iseulde13 months ago

comment:23 @iseulde13 months ago

.12

  • Make sure the background colour for tools is only applied to the main editor.
  • Fixes wrongly calculated toolbar width for smaller screens.
  • Sets the position of the toolbars based on the editor's height instead of the scroll position when unpinning the toolbar as you scroll down. This makes sure it doesn't end up in a weird place when scrolling down too fast.

comment:24 @bradyvercher13 months ago

Should editor-expand.js only be enqueued for post types that have the editor feature support enabled? Otherwise, it's throwing a bunch of JS errors:

TypeError: $top.parent(...).offset(...) is undefined in editor-expand.js:270

comment:25 @iseulde13 months ago

Right, we should add if ( post_type_supports( $post_type, 'editor' ) ) {}...

comment:26 @helen13 months ago

In 29075:

Editor scrolling:

  • Make sure the background color for tools is only applied to the main editor.
  • Fix toolbar width for smaller screens.
  • Ensure toolbar doesn't end up in a weird place when scrolling down too fast.
  • Avoid JS errors for post types that don't support the editor.

props avryl. see #28328.

@iseulde13 months ago

comment:27 @Ipstenu13 months ago

The new editor changes have conspired to make it insanely slow on the iPad. I compared beta to 3.9.1, and it just HANGS all over the place, especially when scrolling. It's pretty near unusable. Actually it made CHROME on the ipad unusable, hanging all over until I force-quit it and reopened. Which is impressive.

3.9.1 works fine. Everything else besides the editor page works fine on 4.0. As soon as you go in to edit posts, it's like you're on a 14.4 modem. Tested on a clean 4.0-beta site to be sure.

comment:28 in reply to: ↑ 21 @iseulde13 months ago

Replying to avryl:

We know there are still quite a few issues for smaller screens. We also still need to disable this for mobile devices.

And unfortunately, I do not have an iPad...

comment:29 @Ipstenu13 months ago

Want my old iPad 1? :) The reason I specifically mention it though is that it's well past 'issue' and right into hands down unusable. Issues I expect, like staggered scrolling and such. Hanging to the point that I have to force quit is amazingly bad.

If there's any debugging I can help with, let me know. I really love posting from my iPad, since I can't do custom excerpts and other meta data bits from the iOS app.

comment:30 @azaozz13 months ago

In 29117:

Editor scrolling: disable on mobile devices and hide the resize handle. See #28328.

comment:31 @Toru13 months ago

Toolbar ends up in weird place when scrolled up too quickly - with or without content in the editor field.
Screencast on Mac+Chrome. http://quick.as/vkwkurvx
(It was same on Mac+Safari )

I guess scrolling by flicking on tablet is probably faster than scrolling on desktop. Perhaps Ipstenu's "HAANGS" and "staggered scrolling" on not iPad is a same issue?

Also, triangle resize handle is gone too.

comment:32 @afercia13 months ago

hi,
one small thing:
I did find useful sometimes to select and copy the "Word count" and "Last edited" details text in the info bar at the bottom of the editor. This isn't possible anymore since the upgrade to TinyMCE 4, when the whole "bar" (table element) became the resize "handle" and preventDefault() used in post.js doesn't allow to select text. Previously, the handle was just the small icon on the right (the last table cell #content-resize-handle) and so that text was selectable. See what changed in #24067
Now that the resize handle is disabled it would be nice to allow text selection again. See attached proposed patch.

A couple of CSS things for Distraction Free Mode:

  1. on larger screens, as soon as you switch to DFW mode you will get the horizontal scrollbar in your browser, see attached screenshot. That's because the width of "#content-textarea-clone" is too large. If you play with "Visual" and "Text" buttons the horizontal scrollbar disappears because it gets calculated again but initially it's too large. I guess "#content-textarea-clone" must be invisible but still have layout to run calculations on its size and offset so it's hidden with visibility and not with display: none. But in DFW mode it's probably safe to completely hide it.
  1. top padding on .mce-edit-area and .wp-editor-area should be reset when in DFW mode. Sorry I didn't find a better solution than using !important.

@afercia13 months ago

comment:34 @iseulde13 months ago

I know there are still a lot of problems with this.

  • Inline styles should be reset when using DFW.
  • On smaller screen, the toolbars don't pin/resize properly, especially when you uncollapse the menu. (Resize the screen to test.)
  • The publish/save button is not visible all the time. I already talked to Helen about that a few days ago. But how do we fix this? Pin the publish box until the bottom of the editor is reached? Not ideal, imo. Move the buttons to the bottom bar (float both word count and save info to the left, buttons to the right)? Somehow add them to the top?
  • Currently, keyboard navigation should work well in TinyMCE once patch .13 is added. The text editor still needs some work... That involves temporarily cloning the selection to the text area clone, calculating the offset of the range, check if it's between the toolbars and if not, scroll the selection into the viewport.
  • Switching from the text editor to the visual editor sometimes triggers autoresize too early.

comment:35 @hardeepasrani13 months ago

Can we do something which will allow the editor to float only when we have clicked on it (when we are writing)?

Like - if we have clicked outside of the TinyMCE, it shouldn't float like it does. Enjoy the final game :)

@iseulde13 months ago

comment:36 @iseulde13 months ago

.15 extends .13. It will scroll the cursor into the "viewport" (between the toolbars) when it moves out of it, but this time it scrolls to the top or bottom, not the middle of the screen, depending on the the key used and where it was before. This also fixes #28882.

comment:37 in reply to: ↑ 19 ; follow-up: @afercia13 months ago

Replying to azaozz:

In 29049:

Enhance the editor on the Add/Edit Post screens, first run. Props avril, see #28328.

sorry just one small thing, now when I'm in the content area of the editor and I press Alt + F11 I have no visual feedback the status bar "path" is focused. See .mce-path-item:focus in editor.css
Reference: http://www.tinymce.com/wiki.php/Accessibility

@iseulde13 months ago

comment:38 @azaozz13 months ago

In 29180:

Editor scrolling: also disable in IE < 9. See #28328.

comment:39 @azaozz13 months ago

In 29185:

Editor scrolling: on setcontent, resize the editor after some delay to allow the browser to render the content. Props avryl, see #28328.

comment:40 @azaozz13 months ago

Seems the changed on keyup in 28328.16.patch breaks the scrolling when the caret "runs off" the bottom of the viewport while typing a long paragraph.

Last edited 13 months ago by azaozz (previous) (diff)

@iseulde13 months ago

comment:41 @iseulde13 months ago

Right, completely forgot about that case. We can safely remove && ( key === VK.DOWN || key === VK.RIGHT )? Updated the patch.

@iseulde13 months ago

comment:42 in reply to: ↑ 37 ; follow-up: @azaozz13 months ago

Replying to afercia:

...when I'm in the content area of the editor and I press Alt + F11 I have no visual feedback the status bar "path" is focused.

The browser automatically does "scroll into view" for any focused element. This works in Chrome as it scrolls that element to the middle of the viewport but in FF and IE it scrolls it to the bottom and the MCE path remains hidden behind the lower bar.

One way to fix this may be to detect these scrolls (if not too complex) and scroll a bit more. If not, will try to find another workaround/fix.

comment:43 @azaozz13 months ago

In 29186:

Editor scrolling: better "scroll into view" in the visual editor. Don't override MCE path focus style. Props avryl, see #28328.

comment:44 in reply to: ↑ 42 @afercia13 months ago

Replying to azaozz:

...and the MCE path remains hidden behind the lower bar...

yep, didn't notice that. Thx for removing the focus style override :)
Noticed that Ctrl+Home (on Windows) doesn't work correctly, it should move the caret to the start of the document *and* scroll to top: the caret gets moved but the editor doesn't completely scroll to the top.
What about something like in the attached example patch? Sorry can't test on a Mac but TinyMCE should already take care of environments with ctrlKey, right?

@iseulde13 months ago

comment:45 @toscho13 months ago

Please make it very easy to turn it off. Right now, it breaks the translating workflow, because you need both editors, original and translation, below each other on the same screen to work efficiently. This affects all translation plugin which offer translations on the same page.

comment:46 @azaozz12 months ago

In 29279:

Editor scrolling:

  • Improve TinyMCE resizing when a floated block is at the end of the content.
  • Improve setting the padding/margin under the toolbar on loading.
  • Add custom event on TinyMCE resizing and use it to adjust the pinning (if needed).

Part props avryl, see #28328.

comment:47 @jadpm12 months ago

+1 on toscho comment: this needs an easy way to be turned off.

The plugins I work with do some things on the post edit screen, like adding help boxes between the title textfield and the editor, adding buttons to the container where the media button is, showing and hiding the editor based on some conditions and also adding syntax highlighting to the editor content. We went from working on 3.9.1 to some glitches during this dev cycle to key functionality broken on the last released beta. I know that some other plugins doing similar things will have the same problems.

By now, on the post edit screens where we need those features, we are dequeueing the editor-expand script, which seems to revert things to the 3.9.1 point.

comment:48 @azaozz12 months ago

In 29336:

Editor scrolling:

  • Add a Screen Option to turn it on/off, and on()/off() methods from JS. Store the user preference.
  • Fix delayed calls to resize() in the TinyMCE autoresize plugin.

See #28328.

comment:49 @azaozz12 months ago

In 29352:

Editor scrolling: run one more adjust() 200ms after scroll or resize in case the browser is slow to re-calculate the element heights and pin/unpin the toolbars. See #28328, fixes #29059.

comment:50 @markjaquith12 months ago

I don't think the min height should have any pinning. Tried messing with that, but there are a bunch of magic numbers that all affect each other. We need these numbers move into variables so that we can play with it.

comment:51 @helen12 months ago

Related (caused by this): #28893

comment:52 @ircbot12 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:53 @WraithKenny12 months ago

.wp-autoresize probably either shouldn't be applied when not in "distraction-free" or shouldn't !important override the tinymce body padding.

comment:54 @helen12 months ago

  • Keywords needs-patch added; ui-feedback ux-feedback needs-testing has-patch removed
  • Severity changed from normal to critical

I think there are a few things we should do/try if we are to keep this in 4.0. I discussed this with markjaquith and nacin in person at WCNYC to general agreement.

  1. Do not pin the toolbar when we are at the minimum height. Right now when you have no text or just a couple lines, the top toolbar will pin and you can scroll the content off the screen underneath it, which makes no sense. The editor should just start shorter, then.
  2. Pin the side metaboxes (when they're on the side) so they never go off screen completely. When you scroll back up, also scroll up the metaboxes. It does mean that two parts of the screen scroll kind of differently. This might not work well in practice, but we need to at least try it.
  3. (lower priority) Some kind of jump to metaboxes / after content link. This needs some careful language consideration, and may be weird in practice. But again, need to try it.

We also generally need to be deliberate about making sure we've considered varying use cases, whether we do something to the feature that accommodates or we rely on the ability to turn it off. As it is, we know that it's very frustrating when you have long content, and when there are multiple editors on a screen, e.g. in multilingual/localized content situations. There is also that you were previously able to configure your editor height and we are now essentially dropping it by default - not a bad thing, but certainly understandable that it may cause some confusion.

Finally, that screen option is awkward - looks like it's just kind of hanging out, and not sure the language is very meaningful.

comment:55 @DrewAPicture12 months ago

Working on a patch for the screen option weirdness mentioned in comment:54

comment:56 @ircbot12 months ago

This ticket was mentioned in IRC in #wordpress-dev by stephdau. View the logs.

@stephdau12 months ago

Attempting to address comment:54: Implementing do-not-pin and defaulting to a minimum editor size based on the current viewport, makes some number less "magical" by making them variables devs can tweak. See

comment:57 @stephdau12 months ago

Chatted with Helen and Andrew today, and came up with attachment:28328.20.patch, which sounds like it might address:

  • some magic numbers being put into variables devs can tweak (there are still some magic buffer vars, but they're vars).
  • defaulting to a size based on the user's viewport, not a static 300px.
  • as well as the do-not-pin-when-less-than-min-height highighted in comment:54.

You can see the patch's behavior in action in:

Hoping this might help.

Last edited 12 months ago by stephdau (previous) (diff)

@stephdau12 months ago

Improves upon attachment:28328.20.patch by alo experimenting with sidebar (meta boxes) pinning.

comment:58 @stephdau12 months ago

Added attachment:28328.21.patch, which improves on attachment:28328.20.patch by also adding / experimenting with sidebar pinning, as requested in comment:54.

Handles sidebar being both smaller or taller than the viewport.
See video at https://cloudup.com/c210q9kl8op

Last edited 12 months ago by stephdau (previous) (diff)

@stephdau12 months ago

This improves the sidebar pinning 1st implemented in attachment:28328.21.patch, for the new post screen. See next comment for gotchas.

comment:59 @stephdau12 months ago

attachment:28328.22.patch tweaks the sidebar pinning 1st implemented in attachment:28328.21.patch, for the new post screen.

On the other hand, it also highlights a gotcha in that screen, when the editor starts so much smaller than the sidebar potentially can be. See in https://cloudup.com/cZr07eqNrwW how the sidebar "jumps" when being pinned at the bottom at the end of the video. I can potentially even picture a context where a user has so many sidebars that it makes some not reachable in the viewport (though I haven't yet tried).

Reminder: attachment:28328.20.patch has the screen size handling, minus the sidebar pinning being experimented with here, if needed.

@stephdau12 months ago

Improves on attachment:28328.22.patch by not proceeding with bottom pinning when the side bar is taller than the left side column.

comment:60 @stephdau12 months ago

attachment:28328.23.patch fixes the issue in the previous patch highlighted in comment:59, when the sidebar is taller than the left-side column.

See https://cloudup.com/clBIzypzNlX

Minor: I note that the sidebar changes width a bit when scrolling, but I can't figure out why.

comment:61 @stephdau12 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

comment:62 @ircbot12 months ago

This ticket was mentioned in IRC in #wordpress-dev by stephdau. View the logs.

@azaozz12 months ago

comment:63 @azaozz12 months ago

In 28328.24.patch: expand on 28328.23.patch, use a class for the sidebar pinning, if the sidebar is taller than the viewport, pin the top or bottom depending on the direction of scrolling.

Last edited 12 months ago by azaozz (previous) (diff)

comment:64 @mrwweb12 months ago

Small thing, but I prefer Gmail's implementation of this feature that leaves a bottom margin below the editor when scrolling. It feels less like the editor is "colliding" with the bottom of the window and I think it just generally looks better.

Screenshot: https://drive.google.com/file/d/0B_1KVBcaBSxMMFNEWjZnbTFfckU/edit?usp=sharing

@azaozz12 months ago

comment:65 @azaozz12 months ago

28328.25.patch improves on .24.patch, fixes cases where the sidebar is taller than the document (not pinned) and leaves some space for the footer.

comment:66 @ircbot12 months ago

This ticket was mentioned in IRC in #wordpress-dev by azaozz. View the logs.

@azaozz12 months ago

comment:67 @azaozz12 months ago

28328.26.patch restores the initial editor height. Also some precommit cleanup.

comment:68 @stephdau12 months ago

I'm still having an issue in attachment:28328.26.patch with the sidebar at the end of scrolling towards the bottom, when the editor is pretty "tall": https://cloudup.com/cfm6cX0-ihY

I'll see if I can figure it out.

comment:69 @stephdau12 months ago

I'll document what I found about the last documented issue (sidebar position), since I'm having trouble dealing with the issue. Maybe someone else will have a clever trick. :)

The reason the sidebar ends up badly positioned, when scrolling down (Chrome on OSX, in my case), is because when the scrolling stops, the browser triggers a lone event calculated as a scroll up by our code. Scrolling momentum effect, maybe?


That triggers L53 in attachment:28328.26.patch

 	353 } else if ( scrolledSide && windowPos < lastScrollPosition ) { 
 	354     scrolledSide = false; 
 	355	   $postboxContainer.css( 'top', '' ); 
 	356 } 

comment:70 @stephdau12 months ago

Proving my point more so than fixing the issue, calculating scrolling based on 2 pixels moves rather than 1 makes the issue less prominent (but still one when a user scrolls passed the end of a document, "thanks" to scrolling elastic/momentum effect).

if ( fixedSide && windowPos > lastScrollPosition + 1 ) {
	// Scrolling down
	sideScrollTrigger = ( $sideSortables.height() - ( windowHeight - 56 ) );

	if ( ! scrolledSide && sideScrollTrigger > 1 && windowPos > sideScrollTrigger && windowPos > $wrap.offset().top ) {
		scrolledSide = true;
		$postboxContainer.css( 'top', -( sideScrollTrigger - 36 ) );
	}
	console.log('down', fixedSide, scrolledSide, windowPos, lastScrollPosition );
} else if ( scrolledSide && windowPos < lastScrollPosition - 1 ) {
	scrolledSide = false;
	$postboxContainer.css( 'top', '' );
	console.log('up', fixedSide, scrolledSide, windowPos, lastScrollPosition );
}

Leads to: https://cloudup.com/cauajcaOEqi

Last edited 12 months ago by stephdau (previous) (diff)

@stephdau12 months ago

same as Andrew's attachment:28328.26.patch, but with fixed sidebar positioning when reaching the end of the document.

comment:71 @stephdau12 months ago

Nailed it! In a non-hackish way, at that. :)

I've added a condition to the scroll-up case to make sure that the current scroll position is inferior to half the height of the sidebar before positioning it back to the top of the viewport. That makes it only do that once the user is half-way up/down the window, and gives it what I find to be a smoother, more natural behavior.

See attachment:28328.27.patch

} else if ( scrolledSide && windowPos < lastScrollPosition && windowPos < $sideSortables.outerHeight() / 2 ) {

Video: https://cloudup.com/ccJc6w0qvm0

comment:72 @stephdau12 months ago

For the record, there some new "magical numbers" that made it in as of .24.patch through to my own .27.patch that should be made into variables dev can tweak as time goes, before this gets committed. 56, 36, etc. :)

comment:73 @stephdau12 months ago

To be tested thoroughly: is there any context in which we end up auto-scrolling the sidebar and make it that some of the meta boxes in the sidebar are in fact never in sight? That's my only concern with the sidebar behavior implemented since .21.patch. Or similar cases where we need to disable auto-scrolling, as we do in some context (editor smaller than sidebar, for one).

Not a show stopper for the feature itself, imho, but has to be addressed pre-RC1.

Last edited 12 months ago by stephdau (previous) (diff)

comment:74 @azaozz12 months ago

...there some new "magical numbers" that made it in

It's just one: 56. That corresponds to the pinned $tools top offset, however we need to know it before they are pinned as the sidebar pins first. Don't see a way to calculate that with JS. We can move it to a var, but it is also used in the css, can probably add a comment that both places need to be edited at the same time. The 36 is 56 - bottom margin for the pinned sidebar.

Yep, .27.patch works well :) Thinking we may want to use fixedBottom instead of $sideSortables.outerHeight() / 2 in case the editor is quite tall. I'll run some tests. Also looking at skipping the sidebar auto-scroll on the first scroll down (when it gets pinned).

comment:75 @stephdau12 months ago

magic numbers: there's also a lone -( sideScrollTrigger - 36 )

fixedBottom: great idea!

comment:76 @stephdau12 months ago

Been trying a few things of my own, and the more I think of it, the more I'm thinking we need to make the sidebar scroll only proportionately to how tall it is compared to the viewport, so that that there can never be unreachabe areas. I almost hate to employ the now-overused term, but it's basically what parallax is, based on the sidebar's height.

EG of unreachable meta boxes (as of .27.patch, which Andrew was tweaking, so maybe no longer an issue): https://cloudup.com/cXPVUDPjtfB

@azaozz12 months ago

comment:77 follow-up: @azaozz12 months ago

In 28328.28.patch:

  • Don't pin the sidebar if the editor is not at least twice as tall as the viewport. We may need to adjust that ratio a bit but generally no point in pinning it if there is no much vertical scrolling. Side effect: it will pin at some point when the user types and exceeds the trigger height and scrolls.
  • Don't auto-scroll the sidebar on the first scrolling down after it has been pinned. That fixes the behavior of auto-scrolling the sidebar almost as soon as it's pinned.

...make the sidebar scroll only proportionately to how tall it is compared to the viewport

Yeah, was looking at that/testing but couldn't get it to work well. Will give it another try. Any ideas on how to get that working well are very welcome :)

comment:78 in reply to: ↑ 77 @stephdau12 months ago

Replying to azaozz:

...make the sidebar scroll only proportionately to how tall it is compared to the viewport

Yeah, was looking at that/testing but couldn't get it to work well. Will give it another try. Any ideas on how to get that working well are very welcome :)

I'll be frank, it's above my math-paygrade. ;)

But the way I understand it: if I scroll 1/3 of the way down in the document, the sidebar should only scroll 1/3 of the way down inside the viewport.

So say:

$sidebarHeight = 1000;
$documentHeight = 1100;
$viewportHeight = 600;
$sidebarTop = 0; // top, which it's not IRL but bear with me
$windowPos = [current position];

onscroll:
$sidebarTop = abs( $viewportHeight / abs( $documentHeight / $windowPos ) );
Last edited 12 months ago by stephdau (previous) (diff)

comment:79 @stephdau12 months ago

Note: The above pseudo code is of course based on the sidebar always being smaller in height than the document itself, which as it turns out might be a bad assumption. I think there is a case in which we reposition the sidebar top above what is the document's top (0), when we're pinned at the bottom, which ends up reducing what the browser defines as the document's height. So we end up with a document height of, say, 800px tall, but a toolbar 1000+px tall. That'd break that pseudomath all to pieces if not handled. :)

@azaozz12 months ago

@azaozz12 months ago

comment:80 @helen12 months ago

In 29495:

Editor scrolling:

  • Reduce the starting height of the editor to better match the height at which the top toolbar unpins.
  • Pin the side metaboxes so they do not get lost when the editor content is long.
  • Turn magic numbers into variables.

props stephdau, azaozz. see #28328.

comment:81 @helen12 months ago

  • Keywords has-patch needs-testing removed
  • Severity changed from critical to normal

The side metabox pinning feels a little weird on first use, but give it a couple tries. There are likely edge cases and use cases that are not accounted for or are not well-served - please report these.

Known issue: the starting height is just a little too tall. Ideally the top bar never pins when the content fits within that minimum height.

Did not tackle the jump link here, as it's lower priority. Opportunity for somebody to jump in with a patch, however.

comment:82 @ocean9012 months ago

Nice work so far!

I'm only seeing an issue while creating a new post. The editor height jumps around: First height seems to be from the user setting, then it jumps to the editor min height (300px?) after that the editor is pinned to the bottom and at last it has the min height again. Tested in Chrome on OSX.

comment:83 @azaozz12 months ago

In 29523:

Editor scrolling:

  • Properly handle change of sidebar height when opening, closing or hiding postboxes.
  • Add a flag when to start pinning. Set it to few pixels more than the initial editor height.

See #28328

@DrewAPicture12 months ago

@DrewAPicture12 months ago

comment:84 @DrewAPicture12 months ago

I agree with @helen in comment:81 that the pinned meta boxes concept is a little weird at first. However, once you realize that it's meant to keep the side boxes in view regardless of how long your page is, the benefit is far more obvious.

Case in point, see 39_unpinned.png vs 40_pinned.png above.

comment:85 @johnbillion11 months ago

A few follow-up tickets relating to the pinned editor: #29225, #29226, #29227, #29293.

P.S. the pinned sidebar meta box behaviour is very nifty. Good work everyone.

comment:86 @ocean9011 months ago

  • Resolution set to fixed
  • Status changed from new to closed

Moved comment:82 to its own ticket: #29307.

Lets call this one fixed.

comment:87 follow-up: @KingYes11 months ago

  • Version set to trunk

Hey all. I'm theme author and I use with wp_editor in my custom Model. But when you add this feature, I get problem with this.

How I can disable this from wp_editor() args?

comment:88 in reply to: ↑ 87 ; follow-up: @azaozz11 months ago

Replying to KingYes:

What kind of problems do you see? This is only loaded on the Edit Post screen. There were couple of CSS changes in editor.css but moved them to wp-admin/css/edit.css in the last patch yesterday night.

comment:89 in reply to: ↑ 88 ; follow-up: @KingYes11 months ago

Replying to azaozz:

Replying to KingYes:

What kind of problems do you see? This is only loaded on the Edit Post screen. There were couple of CSS changes in editor.css but moved them to wp-admin/css/edit.css in the last patch yesterday night.

I have a Builder in this screen. and When I display the editor in my Model, It's breaking the style when I get more content. Do you want to see a Screenshot?

comment:90 in reply to: ↑ 89 @azaozz11 months ago

Replying to KingYes:

Yes, a new ticket and a screenshot would be helpful :)

comment:91 @nacin11 months ago

In 29588:

Screen: Move editor scrolling screen option to the proper place.

see [29336], see #28328.

comment:92 @slackbot9 months ago

This ticket was mentioned in Slack in #core by janneke. View the logs.

Note: See TracTickets for help on using tickets.