Editor: Mobile editor redesign #4456

DanReyLop · 2016-03-31T20:31:19Z

I'm working on #4266, the UI revamp of the mobile post editor. This is by far the most complex task I've started yet, so I'm submitting what I have for now to have early feedback.

The main problem here is that with the new UI (see #4266), desktop and mobile are diverging more and more. I'm trying to tackle it by splitting the editor and sidebar into even smaller components.

What is done:

Changed the mobile action bar to the new 'tab navigation'. Since with the new design the sidebar will be no longer be a sidebar per se, but another tab at the same level as the editor, I've removed the sidebar open/close animation.
Extracted the whole editor sidebar to its own component.
Extracted the "Publish" button to its own component, since it will have to be rendered both in the sidebar (desktop) and in the navigation header (mobile).
Some cleaning of the EditorSidebarHeader component, since now it's going to be a desktop-specific component.

The main UI changes are not yet implemented, so the "mobile" sidebar is still the old one (just now is rendered as a tab). There's still a lot of work to do, but if you could just take a look and tell me if you see something terribly wrong it would be great, so I don't lose time coding the wrong approach to this.

@aduth Feel free to add who you consider relevant to this PR.

aduth · 2016-04-01T12:04:33Z

I'll plan to take a closer look soon, but immediate thoughts that come to mind:

Can we split this into incremental pull requests? For example, I raised the idea of moving only the action bar to the sidebar as one step.
By "splitting the editor and sidebar into even smaller components", do you mean viewport-specific components? I'm fine with more granular components, but maintaining components for each viewport is already quite a pain.

DanReyLop · 2016-04-01T12:35:42Z

I'll see what I can do, I'll try to do the needed changes to get this into a shippable state before completing the redesign. I won't start with the action bar though. I have half-done the navigation bar so I'll finish that first. I agree it's better to do incremental PR if possible :)

By 'splitting' I mean, for example, the "Publish" button. Before, it was part of the GroundControl component, but with the redesign it's needed both in the mobile navigation bar, and in the desktop GroundControl, so I'm extracting that "tiny" button (which actually has quite a lot of logic) to its own component to minimize code duplication.

aduth · 2016-04-01T12:39:09Z

so I'm extracting that "tiny" button (which actually has quite a lot of logic) to its own component to minimize code duplication.

Ah, in which case, I'm totally on board.

I had also eventually planned to move the sidebar to its own component, so agree with the idea of doing so here.

aduth · 2016-04-01T12:42:42Z

client/post-editor/editor-sidebar/header.jsx

-	const closeButtonAction = page.back.bind( page, allPostsUrl );
-	const closeButtonUrl = '';
-	const closeButtonAriaLabel = translate( 'Go back' );
+const EditorSidebarHeader = React.createClass( {


Could you explain the rationale for moving this from stateless function component to a React.createClass object? If it's for the pure-render mixin, note that react-redux connect includes this functionality in its wrapping behavior (docs).

It was for the PureRenderMixin, I didn't know about that react-redux behavior. That was the only reason, so I will revert this change.

DanReyLop · 2016-04-05T15:44:12Z

Ok, I would say that what I have now it's functionally complete.

The navigation bar is complete, the rest is still pending.

Here are screenshots of the date picker. What do you think? It's a bit weird that the date picker button grows if the date string is long, but I think the information it provides is useful.

DanReyLop · 2016-04-05T15:46:45Z

client/post-editor/editor-ground-control/style.scss

 .editor-ground-control__time-button {
-	border-radius: 0 4px 4px 0;
+	@include breakpoint( ">660px" ) {
+		@include is-primary;


This is awful. If you have a better alternative to making a button primary in desktop but normal in mobile, let me know.

This is awful. If you have a better alternative [...]

It doesn't seem that bad to me, to be honest. Seems the primary styling comes more as a consequence of its positioning adjacent the publish button, not that the date picker button is itself a primary action. However, in terms of implementation, instead of creating a separate mixin, could we simply change this to @extend .button.is-primary; ?

@extend doesn't work inside @media queries. That was my first approach :(

@extend doesn't work inside @media queries. That was my first approach :(

Ah, well, that's a shame, though it at least appears to be an upcoming feature (sass/sass#1050).

Perhaps we'd be better to be more specific with the naming? A global mixin named is-primary doesn't seem exclusive to its application on a button (button-is-primary or is-primary-button come to mind).

Agreed, done.

DanReyLop · 2016-04-06T15:25:02Z

client/post-editor/editor-ground-control/test/index.jsx

@@ -1,49 +1,45 @@
-/* eslint-disable vars-on-top */


I'll rebase and resolve the conflicts on this file when #4550 gets merged.

…nent

…ishButton component

… state.

…ly on layoutFocus instead

apeatling · 2016-06-03T16:42:17Z

@aduth & @mtias: Is there any chance of moving this forward? If it is blocked on a difference in approach then I think we need to close this out and use lessons learned for future improvements.

DanReyLop · 2016-06-03T18:18:33Z

I couldn't find time to work on the remaining comments, I'll do it this weekend. All the discussion is in the other (related) PR: #4686 , so I'm closing this one to reduce the noise.

DanReyLop added the [Status] In Progress label Mar 31, 2016

DanReyLop self-assigned this Mar 31, 2016

DanReyLop added the [Feature] Post/Page Editor The editor for editing posts and pages. label Mar 31, 2016

aduth changed the title ~~[IN PROGRESS] Mobile editor redesign~~ Editor: Mobile editor redesign Apr 1, 2016

aduth changed the title ~~Editor: Mobile editor redesign~~ Editor: Mobile editor redesign (WIP) Apr 1, 2016

aduth reviewed Apr 1, 2016
View reviewed changes

lancewillett added the OSS Citizen label Apr 1, 2016

DanReyLop force-pushed the update/4266-mobile-editor-redesign branch from 3592363 to 57b2415 Compare April 3, 2016 13:38

DanReyLop added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Apr 5, 2016

DanReyLop reviewed Apr 5, 2016
View reviewed changes

DanReyLop force-pushed the update/4266-mobile-editor-redesign branch from 1cd5be6 to b6f2e31 Compare April 5, 2016 20:07

DanReyLop changed the title ~~Editor: Mobile editor redesign (WIP)~~ Editor: Mobile editor redesign Apr 6, 2016

DanReyLop reviewed Apr 6, 2016
View reviewed changes

DanReyLop added 14 commits May 23, 2016 11:45

First approach to the new metadata/editor panels on mobile

93f5ab4

Implemented the switch between editor and metadata

1aebe7f

Extracted the primary editor button (publish/update) to its own compo…

2435867

…nent

Re-applied changes lost in the rebase with master

357afff

Fixed tests, moved the relevant GroundControl tests to the EditorPubl…

d738c4d

…ishButton component

Moved the primary button to the navigation bar, only in mobile

f71bffa

Added the proper styles to the mobile navigation bar

6136f6c

Improved the Date picker button in mobile so it shows the selected date

05f0c35

Implemented the changes requested in the comments of #4456

ddd5885

Fixed white background below the post drawer on mobile

10cf63c

Fixed opening and closing preview leaving the tabs in an inconsistent…

72f10e0

… state.

Renamed canSchedulePost function to canPublishPost to make it more clear

3a72d0e

Removed the state in the EditorMobileNavigation component, make it re…

ae7c1a8

…ly on layoutFocus instead

Make the publish button always blue (primary), even in mobile

ea7ba8d

DanReyLop force-pushed the update/4266-mobile-editor-redesign branch from 19ab894 to ea7ba8d Compare May 23, 2016 11:27

DanReyLop removed the OSS Citizen label May 23, 2016

apeatling added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jun 3, 2016

DanReyLop closed this Jun 3, 2016

matticbot removed the [Status] Needs Author Reply label Jun 3, 2016

DanReyLop added a commit that referenced this pull request Jun 13, 2016

Implemented the changes requested in the comments of #4456

a00fb14

DanReyLop mentioned this pull request Jun 13, 2016

Mobile editor redesign #5987

Merged

DanReyLop added a commit that referenced this pull request Jun 18, 2016

Implemented the changes requested in the comments of #4456

3c43fb5

DanReyLop added a commit that referenced this pull request Jun 29, 2016

Implemented the changes requested in the comments of #4456

9bba301

lancewillett deleted the update/4266-mobile-editor-redesign branch September 2, 2016 16:10

DanReyLop added a commit that referenced this pull request Oct 3, 2016

Implemented the changes requested in the comments of #4456

ac0382b

DanReyLop added a commit that referenced this pull request Dec 19, 2016

Implemented the changes requested in the comments of #4456

5eb15f1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editor: Mobile editor redesign #4456

Editor: Mobile editor redesign #4456

DanReyLop commented Mar 31, 2016

aduth commented Apr 1, 2016

DanReyLop commented Apr 1, 2016

aduth commented Apr 1, 2016

aduth Apr 1, 2016

DanReyLop Apr 1, 2016

DanReyLop commented Apr 5, 2016

DanReyLop Apr 5, 2016

aduth Apr 6, 2016

DanReyLop Apr 6, 2016

aduth Apr 7, 2016

DanReyLop Apr 9, 2016

DanReyLop Apr 6, 2016

apeatling commented Jun 3, 2016 •

edited

Loading

DanReyLop commented Jun 3, 2016

Editor: Mobile editor redesign #4456

Editor: Mobile editor redesign #4456

Conversation

DanReyLop commented Mar 31, 2016

aduth commented Apr 1, 2016

DanReyLop commented Apr 1, 2016

aduth commented Apr 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanReyLop commented Apr 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apeatling commented Jun 3, 2016 • edited Loading

DanReyLop commented Jun 3, 2016

apeatling commented Jun 3, 2016 •

edited

Loading