Editor: Mobile editor redesign (phase 2) #4686

DanReyLop · 2016-04-12T20:24:24Z

This PR includes the following changes on the mobile editor:

Move the action bar to the metadata tab
Move the Site information to the main editor tab, plus the status and the date when the post was published
Make the Noticebar appear on both tabs, and make it disappear when the user changes the tab

Note that this PR follows #4456. The code on this branch (update/4266-mobile-editor-redesign-2) is the complete redesign, but the diff between update/4266-mobile-editor-redesign-2 and update/4266-mobile-editor-redesign is only the changes I've listed above. Please tell me if there is a less confusing approach to managing this :)

cc / @aduth @alisterscott @folletto

alisterscott · 2016-04-13T07:00:40Z

I had a quick look as is and I'm seeing a console error: Uncaught TypeError: this.props.onTabChange is not a function Are you aware of this?

mtias · 2016-04-13T09:36:58Z

client/post-editor/editor-status-label/index.jsx

@@ -73,7 +81,10 @@ export default React.createClass( {
 				aria-label={ this.translate( 'Show advanced status details' ) }
 				aria-pressed={ !! this.props.advancedStatus }
 			>
-				<Gridicon icon="cog" size={ 16 } />
+				{ this.props.advancedStatus
+					? <Gridicon icon="chevron-up" size={ 18 } />


This is a regression, should be cog icon.

It's an downward / upward arrow in the redesign mock-ups. @folletto was this an intended change, or just a copy&paste error or something?

We are talking about this icon:

The design was done before that change went in production. So it's misaligned, and it should be a cog. Sorry about that.

No problem, that's what happens when stuff changes, fixed :)

mtias · 2016-04-13T09:59:57Z

@DanReyLop hey, do you mind including a screenshot on the description showing where this is at for easy reference?

folletto · 2016-04-13T10:14:43Z

Quick screenshots:

With author:

mtias · 2016-04-13T10:58:32Z

client/post-editor/post-editor.jsx

@@ -343,11 +323,26 @@ const PostEditor = React.createClass( {
 								site={ site }
 								type={ this.props.type }
 							/>
+							<div className="editor__site">


This class would be a bit misleading given that it also includes the status. I'd look for a better name.

The main component in that bar is the site, the status is like an small add-on. Also, I'm trying to rationalize my decision because I can't come up with a better name, if you do I'll search&replace it :)

mtias · 2016-04-13T10:59:56Z

I think we should only have one notice and change focus to content. Alternatively, we may want to switch to global-notices for this.

DanReyLop · 2016-04-13T12:31:36Z

@folletto Thanks for the screenshots.

@alisterscott I'm not having that console error. Did you try running make run again?

@mtias I think presenting the notice in the tab where the user is would be less confusing. About the other comments, I'll wait to fix them until we decide if we change the approach to this redesign.

alisterscott · 2016-04-15T05:42:39Z

@alisterscott I'm not having that console error. Did you try running make run again?

All good now thanks! I ran make distclean

folletto · 2016-04-25T12:21:01Z

I left a few comments trying to solve some of the questions. I hope that's enough. :)

Once these are sorted, would be possible to have a list of what's still pending (if anything) in order to get this merged? This is a big change so I feel would be useful to have a "We're here" overview. :)

DanReyLop · 2016-04-30T14:49:46Z

Well, if you agree with the latest changes, there should be nothing pending. I would need to:

Rebase mobile-editor-redesign and mobile-editor-redesign-2 (this one) to be updated with master. Since this one is a superset of the original PR, it would be easier to just use this and discard the other branch.
Change the target of this PR so if goes from mobile-editor-redesign-2 -> mobile-editor-redesign to mobile-editor-redesign-2 -> master. I prefer to do it after it's ready to ship because the resulting PR would be really hard to review.

DanReyLop · 2016-05-23T11:51:40Z

I've updated this PR and #4456 with master. It was not an easy merge, but it appears that everything is working fine. Please don't let this PR rot :)

As I said, this PR and #4456 have a different set of commits to make them easier to review, but they need to be deployed at the same time (or better yet, just deploy this one and close the other).

The tip of this branch has all the code of the mobile editor redesign, so if you're testing locally, it's enough to test this branch.

alisterscott · 2016-05-24T00:36:40Z

You can test this live on real devices using: https://calypso.live/?branch=update/4266-mobile-editor-redesign-2

alisterscott · 2016-05-24T01:42:21Z

I've tested this today:

What determines which 'view' is shown upon loading the editor? When adding a new post, sometimes I saw the edit view, and sometimes the actions view. I would prefer to start on the edit view.
Bug: Featured image cuts across the site title on the edit view, see screenshot below
Bug: Editing the featured image above the editor in the edit view means it doesn't then show in the actions view. See screenshot below
Bug(s): the cursor from the editor shows on the actions screen, and as a result of this, when returning to editor the text is sometimes under the toolbar and can't be accessed - see screenshots below
Bug: in desktop view, when expanding a couple of the sidebar areas if there's not enough vertical room, the back button/drafts loses it's padding - see screenshot below

Featured Image Over Site Title

Featured Image that was edited above editor

Cursor on actions screen

Switching back to edit screen when cursor shown

Desktop back/drafts loses spacing when expanding side areas

…g to register (#5824)

Editor: Enable alt text editing for existing images (feature flagged)

…ishButton component

… state.

…ly on layoutFocus instead

…c) to the metadata tab in mobile screens

… of the mobile editor

… main editor tab

…s expanded

…itor view and it appears in the metadata view

DanReyLop · 2016-06-13T11:39:25Z

Looks like it's not possible to change the target branch of a PR after it's created. I've created a new PR with the @alisterscott comments fixed: #5987

matticbot added the OSS Citizen label Apr 12, 2016

lancewillett added [Feature] Post/Page Editor The editor for editing posts and pages. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 12, 2016

DanReyLop mentioned this pull request Apr 12, 2016

Editor: Mobile editor redesign #4456

Closed

mtias reviewed Apr 13, 2016
View reviewed changes

DanReyLop force-pushed the update/4266-mobile-editor-redesign branch 2 times, most recently from 4c73528 to 19ab894 Compare April 20, 2016 22:32

DanReyLop force-pushed the update/4266-mobile-editor-redesign-2 branch from 390b54b to bdf6ed2 Compare April 20, 2016 22:34

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

DanReyLop force-pushed the update/4266-mobile-editor-redesign-2 branch from ad42159 to 0973d8e Compare May 23, 2016 11:47

mtias 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 May 30, 2016

jonathansadowski and others added 2 commits June 6, 2016 17:00

Push Notifications: Make sure serviceWorker is ready before attemptin…

c1b1911

…g to register (#5824)

Merge pull request #5600 from Automattic/update/media-advanced-alt-edit

b88fa95

Editor: Enable alt text editing for existing images (feature flagged)

DanReyLop added 22 commits June 13, 2016 09:52

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

627a7fa

…ishButton component

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

cd1a447

Added the proper styles to the mobile navigation bar

8f9ea12

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

0783ce5

Implemented the changes requested in the comments of #4456

a00fb14

Fixed white background below the post drawer on mobile

fe6f11e

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

3dfd578

… state.

Renamed canSchedulePost function to canPublishPost to make it more clear

9809d2f

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

79789f6

…ly on layoutFocus instead

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

eb54c61

Moved the action buttons (preview, change visitility, make sticky, et…

ceace73

…c) to the metadata tab in mobile screens

Moved the site indicator to the main editor tab in mobile.

f217af4

Changed the behaviour of the editor Notice so it's show in the 2 tabs…

3073fce

… of the mobile editor

Fixed post-editor test

18d399c

Removed the Home icon in the Site component for mobile

14d6986

Changed the editor notice behaviour in mobile so it only shows in the…

7f32f2a

… main editor tab

Make the status label icon a cog again, instead of an up/down arrow

d421730

Fixed desktop sidebar header shrinking when there are sidebar section…

ffa2043

…s expanded

In the mobile editor, now the featured image can be changed in the ed…

a411931

…itor view and it appears in the metadata view

Force displaying the editor view when the editor is initialized

cdc764e

Fixed the style of the Featured Image on the mobile editor

09a84f6

Fixed TinyMCE editor having wrong padding after changing tabs

d013221

DanReyLop force-pushed the update/4266-mobile-editor-redesign-2 branch from 0973d8e to d013221 Compare June 13, 2016 11:29

DanReyLop self-assigned this Jun 13, 2016

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] Needs Author Reply labels Jun 13, 2016

DanReyLop closed this Jun 13, 2016

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 13, 2016

DanReyLop mentioned this pull request Jun 13, 2016

Mobile editor redesign #5987

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editor: Mobile editor redesign (phase 2) #4686

Editor: Mobile editor redesign (phase 2) #4686

DanReyLop commented Apr 12, 2016

alisterscott commented Apr 13, 2016

mtias Apr 13, 2016

DanReyLop Apr 20, 2016

folletto Apr 25, 2016

DanReyLop Apr 30, 2016

mtias commented Apr 13, 2016

folletto commented Apr 13, 2016

mtias Apr 13, 2016

DanReyLop Apr 30, 2016

mtias commented Apr 13, 2016

DanReyLop commented Apr 13, 2016

alisterscott commented Apr 15, 2016

folletto commented Apr 25, 2016

DanReyLop commented Apr 30, 2016

DanReyLop commented May 23, 2016

alisterscott commented May 24, 2016

alisterscott commented May 24, 2016 •

edited

Loading

DanReyLop commented Jun 13, 2016

Editor: Mobile editor redesign (phase 2) #4686

Editor: Mobile editor redesign (phase 2) #4686

Conversation

DanReyLop commented Apr 12, 2016

alisterscott commented Apr 13, 2016

mtias Apr 13, 2016

Choose a reason for hiding this comment

DanReyLop Apr 20, 2016

Choose a reason for hiding this comment

folletto Apr 25, 2016

Choose a reason for hiding this comment

DanReyLop Apr 30, 2016

Choose a reason for hiding this comment

mtias commented Apr 13, 2016

folletto commented Apr 13, 2016

mtias Apr 13, 2016

Choose a reason for hiding this comment

DanReyLop Apr 30, 2016

Choose a reason for hiding this comment

mtias commented Apr 13, 2016

DanReyLop commented Apr 13, 2016

alisterscott commented Apr 15, 2016

folletto commented Apr 25, 2016

DanReyLop commented Apr 30, 2016

DanReyLop commented May 23, 2016

alisterscott commented May 24, 2016

alisterscott commented May 24, 2016 • edited Loading

DanReyLop commented Jun 13, 2016

alisterscott commented May 24, 2016 •

edited

Loading