Problem/Motivation

Drupal 8 wants to provide a much improved workflow for content creators. One of the most critical screens in this is the content creation page. This issue outlines a plan for implementing a new design for this page.

Latest breakdown of sub-issues for this

Per comment #292, revised per #319. Please work on feature issues first. These are all unblocked now, though 3 and 4 are tied together.

  1. [task] commited: #723392: Tame seven's reset.css
  2. [task] committed: #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently)
  3. [major][feature] committed: #1751606: Move published status checkbox next to "Save"
  4. [major][feature] committed: #1838114: Change node form’s vertical tabs into details elements
  5. [major][feature] committed: #1838156: Implement the two-column layout for the new create content page
  6. [task] Needs work: #1751754: Implement new form style for Seven, based on blueprint mockups.

The main issues that this design wants to solve are:

  1. Make the publishing status of the content item much more obvious: Move the publishing state selection out of vertical tabs and position it closer to the Publish/Save actions. Provide a clear status label in the sidebar.
  2. Provide clearer seperation between content and settings: Make the vertical tabs less screen-space consuming by moving them into a sidebar as an accordion.
  3. Update and expand the visual language of the Seven admin theme to express all of the above in a clear and elegant form.

There’s quite some background to this. We have documented our research and iterated on the design proposal.

What we’d like to build is this:

proposed new design mockup - includes a sidebar that will be placed instead of Vertical tabs and new button styling

From our analysis of the feedback on the research and design proposal, we made a few adjustments to our original proposal. See the discussion points below for the parts that need more detailed design work still.

Remaining tasks *Please see implementation plan below for critical work*

See comment #147 for the results of usability testing, this resulted in the following issues to be fixed:

Further discussion


Implementation plan *updated*

We need help in refactoring this from a prototype patch to a full fledged core patch. We have a first implementation a single core patch, but it needed to be split up into sub-issues in order to review, refactor and commit in more sensible steps. This is what still needs to be done before feature freeze:

  1. #723392: Tame seven's reset.css
  2. #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently)
  3. #1751606: Move published status checkbox next to "Save"
  4. Move the node form’s vertical tabs to collapsible fieldsets.
  5. Implement 2 column layout for node forms using either new Blocks & Layouts system or not, depending on time constraints. A mobile layout was never explicitly designed, but the original core patch implemented a responsive layout. Make sure this works well on small screens as we refactor!
  6. #1751754: Implement new form style for Seven, based on blueprint mockups.

Unfortunately these patches are interdependent – no way around that.

Go!

CommentFileSizeAuthor
#300 createcontent-redesign-switched.jpg119.7 KBmikkopaltamaa
#286 1510532-286-create_content_form--button-type.patch10.25 KBmrfelton
#285 1510532-285-create_content_form-_no_reset-_no_reorg.patch48.11 KBmrfelton
#274 1510532-274-create_content_form-_no_reset.patch67.46 KBkika
#273 new_reset_1.png48.76 KBkika
#273 new_reset_2.png59.4 KBkika
#273 old_reset_1.png48.44 KBkika
#273 old_reset_2.png60.14 KBkika
#262 1510532-262-create_content_form.patch77.76 KBeojthebrave
#260 1510532-260-create_content_form.patch78.36 KBeojthebrave
#255 widescreen-overlay.png193.46 KBry5n
#255 widescreen-no-overlay.png196.86 KBry5n
#255 ipad-simulator.png215.1 KBry5n
#255 iphone-simulator-1.png124.09 KBry5n
#255 iphone-simulator-2.png121.81 KBry5n
#255 header-content.png210.47 KBry5n
#250 create-content-1510532-250.patch77.51 KBry5n
#249 create-content-1510532-249.patch77.49 KBry5n
#245 create-content-1510532-245.patch77.45 KBry5n
#243 1510532-no243.patch74.54 KBry5n
#238 Screen Shot 2012-07-29 at 8.33.42 PM.png124.03 KBry5n
#238 Screen Shot 2012-07-29 at 8.31.45 PM.png120.94 KBry5n
#238 Screen Shot 2012-07-29 at 8.25.39 PM.png159.15 KBry5n
#238 Screen Shot 2012-07-29 at 8.30.23 PM.png210.71 KBry5n
#227 node--create-content-1510532-227.patch4.15 KBry5n
#201 node--create-content-1510532-201.patch29.31 KBfubhy
#200 node--create-content-1510532-200.patch29.32 KBfubhy
#198 node--create-content-1510532-198.patch26.85 KBwebchick
#170 node--create-content-1510532-170.patch28 KBry5n
#170 node--create-content-1510532-170.screen.png97.27 KBry5n
#160 Snap2.jpg89.87 KBdgastudio
#159 node--create-content-1510532-159.patch22.96 KBry5n
#159 node--create-content-1510532-159.screen.png229.61 KBry5n
#158 node--create-content-1510532-158.patch22.71 KBry5n
#158 node--create-content-1510532-158.screen.png223.53 KBry5n
#135 drupal8-tags.png134.43 KBrovo
#127 create-content-publish-buttons-snippets.zip5.67 KBry5n
#128 create-content-page-publish-buttons-screenshot.png86.91 KBry5n
#106 node-create-1510532-106.patch31.04 KBmrfelton
#98 Screen Shot 2012-04-09 at 15.38.57.png50.25 KBmrfelton
#97 node-create-1510532-96.patch25.86 KBmrfelton
#94 SmallScreen.png133.62 KBRobLoach
#94 SmallScreenWithoutOverlay.png144.53 KBRobLoach
#94 UsingBartik.png175.44 KBRobLoach
#93 create_article.png131.54 KBaspilicious
#90 node-create-1510532-89.patch24.14 KBmrfelton
#89 Screen Shot 2012-04-09 at 15.09.41.png65.31 KBmrfelton
#86 Screen Shot 2012-04-09 at 14.57.07.png83.73 KBmrfelton
#84 node-create-1510532-84.patch23.91 KBmrfelton
#79 node-create-1510532-79.patch18.03 KBxjm
#77 node-create-content-1510532-76.patch18.03 KBxjm
#75 node-create-content-1510532-75.patch18.02 KBxjm
#69 node-create-content-page-1510532-69.patch1.67 KBry5n
#63 Screen Shot 2012-04-07 at 7.57.56 PM.png94.14 KBry5n
#63 node-create-content-page-1510532-63.patch12.48 KBry5n
edit-content.png59.88 KBtarekdj
#47 Capture d’écran 2012-04-05 à 12.05.24.png186.4 KBjide
#47 1510532-47-content-creation.patch15.42 KBjide
#42 1510532-create-content-042.patch14.89 KBkarschsp
#36 1510532-35-content-creation.patch13.5 KBjide
#36 Capture d’écran 2012-04-05 à 01.36.38.png194.34 KBjide
#37 Capture d’écran 2012-04-05 à 01.49.03.png184.98 KBjide
#37 1510532-36-content-creation.patch14.25 KBjide
#33 1510532-32-content-creation.patch13.07 KBjide
#33 Capture d’écran 2012-04-05 à 00.39.43.png184.11 KBjide
#30 1510532-30-node.admin_.css_.patch3.55 KBtarekdj
#27 1510532-node.admin_.css_.patch3.55 KBtarekdj
#27 preview.png51.9 KBtarekdj
#24 1510532-content-creation.patch10 KBaspilicious
#22 1510532-content-create-screen.022.patch10.97 KBkarschsp
#15 createcontent-redesign-quic.jpg62 KBmarkconroy
#11 createcontent-redesign-basic-node-1510532.jpg205.25 KBsaltednut
#11 createcontent-redesign-basic-node-small-1510532.jpg89.7 KBsaltednut
createcontent-redesign.psd.zip617.61 KByoroy
createcontent-redesign.jpg234.26 KByoroy
createcontent-redesign-small.jpg48.08 KByoroy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Bojhan’s picture

Issue summary: View changes

Updated issue summary.

Bojhan’s picture

Issue summary: View changes

Updated issue summary.

Bojhan’s picture

Issue summary: View changes

Updated issue summary.

Bojhan’s picture

Issue summary: View changes

Updated issue summary.

Bojhan’s picture

Issue summary: View changes

Updated issue summary.

Bojhan’s picture

Issue tags: +Usability, +d8ux, +d8mux

Updated the summary, and created the relevant discussion tasks.

loophole080’s picture

Moved this comment to the design proposal discussion as suggested.

Was going to post there originally, but it moved more quickly than I expected from discussion to decision, just followed the activity... :-)

Bojhan’s picture

@loophole080 It is a little bit late to chime in, we discussed the fundamentals of this approach in research and design proposals. I would personally suggest you comment in design proposal with these concerns, not because they aren't valid but because most of them stretch beyond what we can effectively discuss in an issue - primarily if you want to discuss a conceptual new idea.

The only quick response I can give. The cons you express are real, it will occasionally mean the sidebar is rather empty (only access to one or two accordions) and scrolling in case of a lot of expanded accordion tabs. That does not necessarily mean this approach is wrong, every design decision also has cons and I hope the ones you express are not the 80% case, ideally the too many case can be countered by #1510566: Allow for accordion tabs to be configured per content type and the to few cannot really be countered other than custom configuration.

xjm’s picture

In addition to what @Bojhan says, I'm fairly confident it would be possible to make the layout responsive so that the sidebar column would do sensible things on mobile devices. (This is actually the first comment I made when yoroy mentioned the issue on IRC.)

Edit: Overall I think the design is great (I said this in IRC but I should probably say so here!) and I agree it would be best to get implementing this, and then iterate on it to address the weaknesses and potential improvements. :)

xjm’s picture

Issue summary: View changes

Updated issue summary.

Bojhan’s picture

Issue summary: View changes

Updated issue summary.

yoroy’s picture

Have a look at http://groups.drupal.org/node/218594 where we explore display of this page on small screens. (added link to issue summary above as well)

Crell’s picture

My only concern (which I know I've expressed previously) is that we make sure that we don't code ourselves into a corner where we assume that all content adding/editing is happening by site admins in an overlay. There are plenty of use cases where nodes, or at least some node types, are edited by mere authenticated users by the thousands who would have access to only a small subset of the fields and fieldsets in this page, and for them the form should appear as any other "normal" page on the site, without fancy sidebars or popups or whathaveyou.

As long as that is still straightforward to do (meaning I can continue to maintain the site that got me into Drupal in the first place :-) ), I'm comfortable proceeding with this approach and letting Bojan and yoroy loose to do their thing. :-)

loophole080’s picture

re #6 I think the scenario of having a role where people only have access to/care about the publishing options is quite a common one. The separated top toolbar for "publishing actions" idea is very handy for such a scenario, as well as the completely separated tab/page for other settings.

The toolbar could simply appear at the top of the content region and be independent of shortcuts or overlay functionality (or extended settings). It would also provide a good indicator that someone is in "editing mode", especially if it was very visually distinct, perhaps with some colour coding to re-enforce the publishing status?

I also found myself asking, even as a full admin, do I need to see all the settings on the same page as the content edit form? Why not separate them and keep the content edit form as clean as possible? Are we really gaining anything having the settings crammed in with the content edit screen?

Is it worth running a survey on this?

(see original post)

Crell’s picture

loophole: My point is that the concept of an "editing mode" is specific to only certain workflows. Common workflows, sure, but not all workflows. I'm saying that I'm fine with proceeding with the plans that Bojan and yoroy have put together *provided that* they are done in such a way that workflows that do not have a distinct "now in editing mode" and "now in viewing mode" split are still straightforward to implement, as there are many of those, too.

svenryen’s picture

is the plan that the content and accordion parts scroll independently or together?

maybe we could consider letting the "Publish" "Preview" etc buttons being displayed at all time and let both columns above scroll independently,
so that the Publish button can be reached and seen easily?

yoroy’s picture

That's why we need patches, no way to tell without proof of concept. Starting point is no fixed positioning of the sidebar. I'm very curious to see what happens when we keep the sidebar in view too.

saltednut’s picture

Regarding crell's comments: I agree that there are plenty of workflows where overlay is simply not an option. Forgive me, but is there a mockup or any discussion regarding this layout and how it is treated as a full page? I imagine it working very similarly, but it will be interesting to see what this ends up looking like when patches come thru.

The mockup shows a large amount of extra fields on the example content type. This somewhat obfuscates what the design would look like for a default page node. Attached is essentially what people would be seeing upon first firing up D8 and editing a basic page. Note that the lengthy sidebar dictates a large amount of negative space below the content editing area. I do not think this would affect UX. However, I do see it as an aesthetic issue that will be noticeable when creating entities of the most basic node bundles.

createcontent-redesign-basic-node-small-1510532.jpg

saltednut’s picture

Upon further consideration, I do think this sidebar will create UX problems. A few that come to mind:

  1. Closed fieldsets assume that the user is familiar enough with Drupal to open them up and edit the Menu/et al. (This has always been a problem. RE: Menu Settings in previous versions of Drupal)
  2. If all fieldsets are expanded, this will call for a considerable amount of scrolling/unused negative space.
  3. Verbose field descriptions on the sidebar now take up to 3-4 line breaks, causing a noticeable disruption between fields.
yoroy’s picture

No fair cutting out the custom content fields and leaving in the big expanded meta-tags in the sidebar :)

David_Rothstein’s picture

Closed fieldsets assume that the user is familiar enough with Drupal to open them up and edit the Menu/et al. (This has always been a problem. RE: Menu Settings in previous versions of Drupal)

I had the exact same thought when I looked at this - at least without the summaries, these look a lot like collapsible fieldsets from Drupal 6, and we know those had some usability problems.

I posted a comment at http://groups.drupal.org/node/217434#comment-727329 discussing this and a related issue further; since this is a design question (or at least somewhere between the boundary of design and implementation?) it may make more sense to discuss over there.

markconroy’s picture

Please excuse the ugliness of my design (it's also my first attempt to add to this type discussion, so apologies).

I like my text area to approximate the size of my screen when the node is published (gives me a better idea of how things will look without having to click "preview" so many times). To this end, I'd like to see the sidebar tabs area removed.

I think a lot of real estate could be saved then by using something like the quicktabs module instead of the vertical tabs module (perhaps a "horizontal tabs" module). This would mean all tabs could be quickly viewed. They would, also, fill right across the screen if the sidebar is removed - so you could put about 6-7 tabs on a line. See the (rather ugly) screenshot:

new create content

Also, perhaps it might be an idea to change the "Publish" button to "Publish Now", the "Save Draft" button to "Save for Later", and "Preview" to "Show Preview".

aspilicious’s picture

A note to everyone in here:

I advise EVERYONE to *read* the issue summary carefully. Once that is done you should realize that the proposed design in the intial post will be implemented. (with possibly some SMALL changes)

OH NOES I HATE THE DESIGN!

We can keep discussing forever, but lets start this issue off the right way - with a patch. That way we can test and make iterative improvements!

Thnx!

webchick’s picture

Yeah, we should keep this issue focused on implementing the design in the OP, to-spec. If there are further comments about the design, probably http://groups.drupal.org/node/217434 is the best place. Further iterations could be iterated on once the initial one is done.

karschsp’s picture

Unless someone tells me not to, I'm working on a (really ugly) patch to at least get us to a point where we can usability test this design, should have something later this evening.

yoroy’s picture

Hooray, that's the spirit. Go karschsp!

RobW’s picture

Very excited about this, I think it's going to be a great step forward. A couple of questions on non-layout goals for this redesign:

1. Is the end goal for site builders to be able to specify which fields go in the content area and which in the settings bar, or will settings be what is now in vertical tabs only?
2. Is this proposal meant for the overlay only, or both overlay and in-primary-theme use?

tarekdj’s picture

Hi,
I think the first thing to do is to put title filed, body filed, additional fileds and action buttons in a container. And put the vertical tab in another container. This will help creating the 2 cols layout. Any idea about coding this ?

karschsp’s picture

Assigned: Unassigned » karschsp
Status: Active » Needs work
FileSize
10.97 KB

OK everyone. Avert your eyes. Just apply the patch and move along. :P

As mentioned in the issue summary, this patch is a first pass just to get us to a point where we can test some of the new design concepts...implementation details to be worked out later. There are some images commented out in the CSS since I'm not really sure how to add images to a patch.

dodorama’s picture

It might not be strictly related but I wonder if it could make sense to have a template file for the node edit page like we have for nodes.

aspilicious’s picture

Status: Needs work » Active
FileSize
10 KB

This one should apply better, and looks a bit more like the OP.
(optimized for overlay)

(code is still ugly and it's optimized for overlay, but it's a start)

yoroy’s picture

Status: Active » Needs review

It would be great if somebody could post a screenshot, it will be interesting to see how we progress towards the design.

Thank you for working on this!

Status: Needs review » Needs work

The last submitted patch, 1510532-content-creation.patch, failed testing.

tarekdj’s picture

Assigned: karschsp » Unassigned
Status: Needs work » Active
FileSize
51.9 KB
3.55 KB

Hi,
this is a preview + patch based on #22 patch. It adds some changes to node.admin.css
it's basicly somme css 3 gradient and shadows

tvn’s picture

Patch from #24 can be seen here: http://dev.test567.gotpantheon.com/
login: testuser
pass: testpass

aspilicious’s picture

tvn his site is based on patch #24.

#27 and #22, the OP doesn't want those save buttons on the right side, please just implement the picture in the summary. AFTERWARDS we can discuss other options.

Thnx!

tarekdj’s picture

this patch adds styles to patch #24

Bojhan’s picture

My feedback is determined from the demo site and the screenshot (neither #24 or #30 patches seem to work for me). In general this is moving exactly in the direction, that we want! This is getting really close to something testable, I would love if we could move closer to the image in the top in terms of styling. Because this allows us not only to test the concepts but also specific affordances (clues) the visual styling gives, and with that influences usability performance.

I would like us to improve on the following items:

  1. Add a small shadow on the left side of the toolbar, now it feels to much "hanging" rather than next to, because of the visual effect it lacks. This will have a big impact on our potential testing.
  2. Add the styling for the buttons, including the "box" background. We want to test if the buttons are more noticed with these style elements.
  3. Remove the tiny bars on the side of activated vertical tabs, a collapsed VT should span the whole width.
  4. Implement the revision collapsing within the sidebar top part, we want to test whether this interaction pattern works there.

Once there is a patch I can apply as well, I will help out with the detailed styling of font-sizes, colors etc. If anyone wants to give a crack at applying the styling to the form elements in the content area, that'd be very helpful.

I am coordinating with dcmistry, who has run up many Drupal usability studies - to get this tested. We are going to develop a plan over the weekend, and immediately start recruiting and scheduling participants.

quartsize’s picture

Is it intended that the sidebar accordion allow multiple panes open simultaneously?

jide’s picture

An updated patch. We're getting closer.
I did not digg so much, but #group property is ineffective for the title "field", which causes some problems (but does the #group property works at all ?).

karschsp’s picture

#group only works on fieldset.

eigentor’s picture

Ah, I'd be a great fan if the save button more often would be blue in Drupal. You just cannot miss it. Kind of the anchor a new user clings to.

jide’s picture

Of course, seems logical now you say it :)
Seeing this :
$form['title']['#group'] = 'edit_primary';
In the code confused me, wasn't sure if it was something new in D8.

I put the title field in $form['edit_primary'] instead for now (Yeah, implementation *really* sucks).

Here is an updated patch / screenshot.

jide’s picture

Last one for today. I also put actions in $form['edit_primary']. This is of course to quickly stick to the mockup without messing with CSS too much, and that's something we do not want when we will implement this, I hope I understood the intent well here.

Crell’s picture

Can we please *not* post giant, over-sized images into every frickin' comment? Just attach them for us to click on, please. Otherwise this page will very very quickly become completely unusable. It already is forcing horizontal scrolling rather badly.

jide’s picture

Yes you're right, sorry.

Crell’s picture

Thanks. Sorry if came off as snippy, but another thread that had large embedded images ended up as several megabytes to download every time someone posted a comment. :-)

jide’s picture

No worries :)

Note: We'll have to remove + dpm($item['href']); introduced in #22.

karschsp’s picture

Here's a patch which adds the revision section below the "node summary" and also removes some gradient backgrounds that were showing up on .node-actions and file form elements. Also removes the stray dpm() that was left in there.

yoroy’s picture

Status: Active » Needs review

This is progessing quickly, the latest screenshot looks great already. I'll have to play around with the actual patch applied to give detailed feedback. From looking at the screenshot, it seems to me that the whole text formats thing is just too much (even for a user/1 scenario) and the text field gradients seem a tad too heavy.

Again thanks to all who's working on this, it's really exciting.

tvn’s picture

Status: Needs review » Active

I've updated demo site with patch in #42
http://dev.test567.gotpantheon.com/
login: testuser
pass: testpass

tvn’s picture

Status: Active » Needs review

oops, didn't mean to change status

Status: Needs review » Needs work

The last submitted patch, 1510532-create-content-042.patch, failed testing.

jide’s picture

Status: Needs work » Needs review
FileSize
15.42 KB
186.4 KB

Update.

@karschsp The gradient in .node-actions was intended, it's like in the mockup.

I used a different technique for placing the sidebar so that it is always expands to the height, without specifying a height in CSS. Fields in sidebar won't be too long anymore too.
There is a weird bug at the top of the overlay which causes an extra padding, but that was here before (although not visible because the were no gray sidebar before !).

jide’s picture

@yoroy Gradient colors were color picked directly from the PSD ;)

Status: Needs review » Needs work

The last submitted patch, 1510532-47-content-creation.patch, failed testing.

Bojhan’s picture

I have removed a few oops comments. Wondering why I dont see a top sidebar when I am creating? It should be displayed on both display and creation. You can usually already fill author, and url could be auto filled or just hidden from view (lets not over-worry about that part just yet).

It seems like apart from that only 1) is an outstanding issue?

karschsp’s picture

I guess some clarification is needed on that top sidebar then. Is that supposed to be "display/informational purposes only"? Or should those values be editable? In the current patch, they are display only. To modify those values you need to click in the Authoring Information and URL Path Settings sections.

If they are intended to be display only (as they are currently implemented), then I'm not sure what value the top sidebar has on node/add forms vs. node/edit.

marcvangend’s picture

I'll review more code later, just some remarks about the CSS for now:

+++ b/core/modules/node/node.admin.css
@@ -10,3 +10,288 @@
+  background: -webkit-gradient(linear, left top, left bottom, from(#f1f1f1), to(#fafafa)); ¶
+  background: -webkit-linear-gradient(top, #f1f1f1, #fafafa); ¶
+  background: -moz-linear-gradient(top, #f1f1f1, #fafafa); ¶
+  background: -ms-linear-gradient(top, #f1f1f1, #fafafa); ¶

Remove trailing spaces.

+++ b/core/modules/node/node.admin.css
@@ -10,3 +10,288 @@
+	-moz-box-sizing: border-box;
+	box-sizing: border-box;
+	max-width: 100%;

Use spaces instead of tabs.

+++ b/core/modules/node/node.admin.css
@@ -10,3 +10,288 @@
+  background: -webkit-gradient(linear, left top, right top, from(#ebebeb), to(#fafafa));
+  background: -webkit-linear-gradient(left, #ebebeb, #fafafa);
+  background: -moz-linear-gradient(left, #ebebeb, #fafafa);
+  background: -ms-linear-gradient(left, #ebebeb, #fafafa);
+  background: -o-linear-gradient(left, #ebebeb, #fafafa);

Has is been "officially" decided that this is the way we create CSS gradients? If I'm not mistaken, there are more browsers we can support. The tool at http://www.colorzilla.com/gradient-editor/ produces the following CSS (cleaned up and re-ordered by me):

background: #ebebeb; /* Old browsers */
background: -webkit-gradient(linear, left top, right top from(#ebebeb), to(#fafafa)); /* Chrome,Safari4+ */
background: -webkit-linear-gradient(left, #ebebeb, #fafafa); /* Chrome10+,Safari5.1+ */
background: -moz-linear-gradient(left, #ebebeb, #fafafa); /* FF3.6+ */
background: -ms-linear-gradient(left, #ebebeb, #fafafa); /* IE10+ */
background: -o-linear-gradient(left, #ebebeb, #fafafa); /* Opera 11.10+ */
background: linear-gradient(left, #ebebeb, #fafafa); /* W3C */
filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#ebebeb', endColorstr='#fafafa',GradientType=1 ); /* IE6-9 */

I'm not sure if the 'filter' rule for IE6-9 works on all elements, so let's test that. I also think it's good to decide what the background color should be in case gradients aren't supported (the "Old browsers" part).

jide’s picture

@marcvangend If I'm not wrong (and I hope I'm not !), goal is not to have a good implementation, otherwise we are completely wrong here, I'm not even sure it shouldn't be in the theme layer, or anyway something completely different. Goal is to test the UI, afterwards we'll see if this is worth implementing well. So patch reviewing may be useless here. Otherwise I would be totally ashamed of the code I wrote :) TBH, it feels weird to write such dirty code for a core patch, but that's the point.

Bohjan, yoroy, can you confirm ?

marcvangend’s picture

@jide, I think we're pretty much agreed there. Maybe I shouldn't have mentioned the spaces and tabs, but I figured that cross-browser compatibility is important when you want to want people to test the UI.

yoroy’s picture

jide: confirmed. I did ask marcvangend to provide some technical feedback, he wisely decided to limit that to some CSS issues :)

jide’s picture

Hehe, Yes, indeed, makes sense then.

We should try and see if filters are a good option instead of a background picture fallback, filters can cause weird problems in some situations.
BTW, I'm surprised the tool does not generate the standard "-ms-filter" too (it should !).

And fixing tabs shouldn't be that hard :)

RobW’s picture

IMO good css shouldn't really matter at this point. As devs, most of us testing will have a class A browser, and if we're a UI/X tester with a group of clients or test users we'll probably have control over the environment enough to specify a browser. Plus, implementation may change the underlying html structure, and then we'll need to revise css and all the time spent on little visual and cross browser tweaks will be lost. Dirty prototypes first, then structural and backend implementation, then design and theming (which is where I'd like to jump in)!

Progress looks great so far, and so fast! Nice work karschsp, aspilicious, tarekdj, and jide.

RobW’s picture

@#32: Yes, I think that multiple sidebar accordions should be expandable at once. There are settings that inform each other, and a user might want to look at multiple or copy and paste between.

markconroy’s picture

this is excellent stuff. Well done. I take away what I said above with my (ugly) preview thing.

nevets’s picture

Why treat taxonomy as special since it is a field in Drupal 7 and I like the ability to place it on the form where I want instead of the system deciding where it goes

ry5n’s picture

Overhauled sidebar css, checking against admin screen without overlay, checking across browsers (latest Chrome, FF, Opera, IE + IE8). I’ve deviated very slightly from the mockup in terms of sidebar styling (sorry @webchick), but this can easily be changed to match if not deemed an improvement.

Still needs lots of work. Note two things:

  1. Added functionality to the collapse.js in core/misc by added an .expanded class for the expanded state. Not used in this patch, but could be during future refactoring
  2. This is my first shot at a patch for core – go easy! :)
xjm’s picture

Thanks @ry5n! That screenshot looks great.

The patch in #63 could use some cleanup to meet our coding standards. Few specific points:

  • There should not be trailing whitespace on the ends of lines.
  • Lines should be indented by two spaces only.
  • Properties should be ordered alphabetically.
  • Comments should begin with a capital letter and end with a period.
  • Let's not use comments like "end of foo" or "rulers" of special characters (e.g. -----------).
  • We should not leave commented-out code.

Following these standards will make the code easier to review. Reference: http://drupal.org/coding-standards

Also, when you submit a new patch, you can set it to "needs review" so that it is automatically tested by our testbot. (For a CSS patch, this mostly makes sure it applies.) It can be set back to "needs work" afterward. Thanks!

ry5n’s picture

@xjm Thanks for your patience re: coding standards. I’ve read through them now (silly of me not to have done before) and will submit an updated patch shortly.

David_Rothstein’s picture

I thought based on the above that this issue wasn't going to worry about code reviews or coding standards yet (since it's still in the prototype phase)?

Not that it hurts to follow them, of course :)

webchick’s picture

Issue tags: +prototype phase

Right. Making up a "prototype phase" issue tag to reflect this. The point of this patch currently is only to get something we can touch and play with. Its goal isn't (yet) to be core-worthy. That'll come later, after we use this prototype to gather some data about user behaviour.

webchick’s picture

Issue summary: View changes

link to mobile version mockup

webchick’s picture

I've also updated the issue summary to reflect that, since this is at least the 2nd or 3rd time that clarification has been made.

ry5n’s picture

Status: Needs work » Needs review
FileSize
1.67 KB

Even though this is for prototyping, I though I may as well clean up the stylesheet to better match coding standards (patch content is identical). BTW, I’m not sure if I should be marking this `needs review` or just `needs work`.

Status: Needs review » Needs work

The last submitted patch, node-create-content-page-1510532-69.patch, failed testing.

tvn’s picture

I've updated demo site with patch #47. Layout does not break on Menu settings anymore which is great.

Both patches #63 and #69 didn't work for me.

tvn’s picture

Issue summary: View changes

Updating the issue summary to reflect that this is just a prototype patch. (webchick)

Bojhan’s picture

@ry5n Congrats on your first core patch! Could the latest patch be updated, I am trying to apply it but can't get it to work?

It looks like we are a lot faster than my estimation, which is super good news! We are going to post testplan in the next few days, and invite everyone to help out usability testing this!

There is still one outstanding to do, the meta information in the top of the sidebar is not shown on creation. But only on editing, this is wrong, it should also display information on creation (Publishing state can be filled, Author can be filled - URL should probably update on AJAX?).

xjm’s picture

@ry5n, looks like that patch is against your previous commit maybe? Perhaps that is also why your patch in #63 did not apply. See http://drupal.org/node/707484. If you'd like help creating the patch, you can join us in IRC in the #drupal-gitsupport channel. Thanks!

xjm’s picture

Assigned: Unassigned » xjm

Actually I just tested and confirmed that #63 and #69 are interdiffs. Rerolling them into a new patch.

xjm’s picture

Assigned: xjm » Unassigned
Status: Needs work » Needs review
FileSize
18.02 KB

Attached is #47 combined with #63. Also tried to clean up the whitespace, etc. so that the patch is easier to review.

Status: Needs review » Needs work

The last submitted patch, node-create-content-1510532-75.patch, failed testing.

xjm’s picture

Status: Needs work » Needs review
FileSize
18.03 KB

Missed a few.

Status: Needs review » Needs work

The last submitted patch, node-create-content-1510532-76.patch, failed testing.

xjm’s picture

FileSize
18.03 KB

Rebased.

xjm’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, node-create-1510532-79.patch, failed testing.

xjm’s picture

Those are the correct fails. :) Carry on. Thanks again @ry5n!

tvn’s picture

Status: Needs work » Needs review

#75 looks good, only lets remove white border on the right of sidebar to bring it closer to original design

mrfelton’s picture

FileSize
23.91 KB

Try this one. I've made several adjustments to bring it more inline with the original visual, including:

- adjustments to font sizes in the sidebar
- adjustment to the gradient in the sidebar
- move the field description above the field
- remove the white border on the sidebar
- Adjust the menu collapsed arrow icons so they are larger and grey

Bojhan’s picture

@mrfelton Can you make a screen? Did you go from #79?

I would love if anyone could fix the problem from comment #72 of top of the sidebar showing nothing on creation.

mrfelton’s picture

Yes, I went from the patch at #79. Here is a screenshot:

mrfelton’s picture

Note, I'm not sure why the turned down arrow is showing the small icon still. I updated that image too. I think its a local caching thing.

mrfelton’s picture

@Bojhan re the problem outlined in#72, that looks like a completely new tab, right? It's not quite the authoring information, but not quite the publishing options either, but sort of something in the middle. Or, is it supposed to be a complete combination of the authoring information and publishing options tab?

mrfelton’s picture

Slightly revised patch, ensures that text fields take up the full width.

screenshot

mrfelton’s picture

FileSize
24.14 KB

And the patch...

Bojhan’s picture

@mrfelton The changes are looking great! When you edit a page, you see some "meta information" in the top part of the sidebar just as in the issue summary. I basically want that on the creation on content too (I don't think we should go to the trouble of combining stuff, just yet). Lets just make sure we have the same top area on creation as on edit.

I have removed your image from displaying straight into the issue, to keep this issue scan able. It's not a standard practice, but for this issue it makes it easier for people to read.

aspilicious’s picture

Use this for your summary

    $node_summary = '<div class="node-summary"><h3>' . $published . '</h3>';
    $node_summary .= '<p><em>Last saved ' . format_date($node->changed) . '</em></p>';
    $node_summary .= '<ul><li><strong>Author</strong> ' . $node->name . '</li>';
    $node_summary .= '<li><strong>URL</strong> ' . drupal_get_path_alias('node/' . $node->nid) . '</li>';
    $node_summary .= '</ul></div>';
  } else {
    $node_summary = '<div class="node-summary"><h3>Not published</h3>';
    $node_summary .= '<p><em>Not saved yet</em></p>';
    $node_summary .= '<ul><li><strong>Author</strong>*the name of the current user*</li>';
    $node_summary .= '<li><strong>URL</strong>*some url*</li>';
    $node_summary .= '</ul></div>';
  }

The text between * needs to be dynamic ;)

aspilicious’s picture

FileSize
131.54 KB

Example

RobLoach’s picture

FileSize
175.44 KB
144.53 KB
133.62 KB

I like where this is going, would making it responsive first help? Here's some attached screenshots.

yched’s picture

Field API 'textifeld' widgets have a setting specifying the desired width of the input. Could we make sure that it doesn't get overriden by some hardcoded 100% CSS rule ?

Status: Needs review » Needs work

The last submitted patch, node-create-1510532-89.patch, failed testing.

mrfelton’s picture

Status: Needs work » Needs review
FileSize
25.86 KB

This patch adds in the publishing information stubb to the top of new nodes.

mrfelton’s picture

And the screenshot to go with it ^

Bojhan’s picture

Awesome! Oke, I am going to mark this "prototype ready for testing" - and fill you in as soon as possible with a test plan :')

@RobLoach See http://groups.drupal.org/node/218594 , I don't think we should do that in this issue.

mrfelton’s picture

@yched: Not quite sure how to best do that. By default a size of 60 is applied. We can use [size="60"] to only set the width to 100% for that default size, ensuring that if someone changes it from the default 60 that their new size will be respected. Alternatively, perhaps we should remove the default 60. Why 60 anyway? Where did that come from?

Status: Needs review » Needs work

The last submitted patch, node-create-1510532-96.patch, failed testing.

yched’s picture

@mrfelton: not sure where that 60 default size came from, very possibly comes from the early cck days.
Is it possible to target inputs having *no* explicit size attributes ? Then we could set the default size to 'empty' (= leave the default styling for this entity's forms apply) rather than an arbitrary 60.

mrfelton’s picture

yes, I believe we could use :not([size])

RobLoach’s picture

Please hit #1277352: Responsive vertical tabs first, and try making this work for all vertical tabbed pages rather than just a bunch of custom CSS for node editing. This could actually be a detriment to what the Mobile Initiative is trying to accomplish.

aspilicious’s picture

Wait what? why is the node published and NOT saved lol

mrfelton’s picture

Status: Needs work » Needs review
FileSize
31.04 KB

Good catch! That's resolved.

I have also remved the default size of 60 from textfields, and updated the 100% width css so that it only affects elements with no width set. However, in doing this it has become apparent that its not just text fields that set a default size, but other fields also do similar things. For example, number fields set the size to 12, and doesn't give an option to change this. Not sure what, if anything, to do about that

I also spotted what I believe to be a bug in element_set_attributes, which is that attributes get set even if there is no value for them. For example, even after removing the size attribute, an empty 'size' attribute was still being added to the markup. So you would end up with something like this

<input class="text-full form-text" type="text" id="edit-field-test-und-0-value" name="field_test[und][0][value]" value="" size="" maxlength="255">

instead of what I think you should have which is

<input class="text-full form-text" type="text" id="edit-field-test-und-0-value" name="field_test[und][0][value]" maxlength="255">

Attached patch also fixes that, but I don't know if that will break other things or not. I guess the test suite will tell us that.

mrfelton’s picture

@Rob Loach I think the intent here is simply to get a prototype ready for testing. If the implementation is good, and deemed to be user friendly, then there are quite a few things with this patch that would need fixing up anyway. I'd say its more important to get something ready for testing quickly, than it is to ensure that css and other features are implemented in the best way possible or even in a way that is 100% compatible with the work being done in other tickets such as the one you pointed to. Any CSS coming out of this issue can be made more generic after.

Status: Needs review » Needs work

The last submitted patch, node-create-1510532-106.patch, failed testing.

Bojhan’s picture

I have just had a meeting with dcmistry, to discuss the plans for testing. We are now developing a test plan, important dates are:

  • Wednesday, April 11: Initial draft of test plan (will be posted on g.d.o/usability, pending feedback).
  • Friday, April 13: Final draft of test plan, testing can begin.
  • Sunday, April 22: All testing ends, if we have enough participants earlier that works too.
  • Wednesday, April 25: All results have been analyzed, and final results are posted.

We want to test at least 5 beginners (no experience with Drupal), and 5 intermediates (some to a lot of experience with Drupal.

We will definitely need help to test enough participants! Let me know if you are interested in either doing testing, or sending us through participants we can test. It would help us greatly, if more people could test maybe one or two friends.

ryan.armstrong’s picture

I've got some friends who woud be in the group of people who would build a website, but who have never used Drupal before. I'd be more then happy to run a test. What do I need to do? How many people should I get?

tvn’s picture

Patch #106 didn't work for me, so demo site is now on #97.

dcmistry’s picture

Ryan,
Superb! You can help us with testing/ and or recruiting. I should have a solid plan by tomorrow.

Stayed tuned!

stevector’s picture

ryan.armstrong’s picture

dcmistry,

Awesome sound great, I'll keep an eye on this post. Also, feel free to contact me directly as well!

ryan.armstrong’s picture

Quick question regarding the scope of suggestions for this layout. For example, would be it out of scope of this task to suggest that the Authored On field use the jQueryUI date picker rather then then plain text string? Though the format is relatively strait forward (However, it does require an entire paragraph to explain it), I would imagine content authors wanting something more user friendly.

This might be out of the scope of this exercise, but I thought I would just mention it.

stevector’s picture

Ryan, The JQueryUI date picker takes configurable formats. http://jqueryui.com/demos/datepicker/#date-formats

The difficulty is a separate element is then required for time of day. This might be easier if some of date module's functionality get into core: http://groups.drupal.org/node/221229

I'm uncertain on the scope question. I think it could be handled in a separate issue.

RobLoach’s picture

Quick question regarding the scope of suggestions for this layout. For example, would be it out of scope of this task to suggest that the Authored On field use the jQueryUI date picker rather then then plain text string?

#504524: Extend Authored on field with jQuery UI Date Picker ;-)

ryan.armstrong’s picture

Haha, well there ya go, thanks Rob and stevector!

ry5n’s picture

@Bojhan @xjm Thanks for your encouragement :) I was away for a few days but I’m back; changes look good. I won’t be able to assist with user testing this time around but I’m eager to see the results.

Potentially important: one of the usability issues [previously noted](http://groups.drupal.org/node/214898#comment-716094) is confusing labelling of the save button. Solution in the mockup is this:

Does save publish my content?

I wonder if we should make a rush effort to implement this in the prototype before user testing gets under way?

I’m willing to pitch in with some css over the next couple of days.

Bojhan’s picture

@ry5n We are going to start testing on Saturday, if you have some time today/tomorrow to make a patch - then we can probably incorporate the new buttons in the test.

xjm’s picture

This is so exciting! Thanks everyone. Should we make a core group announcement about the testing?

ry5n’s picture

@Bojhan I have some time and can write the necessary markup and CSS no problem, but I’m not well-versed enough with Drupal and PHP to get the new markup into the form and have a working patch – at least not quickly. A more experienced contributor would have to help with that part.

ry5n’s picture

@Bojhan Why don’t I write a static html/css snippet for the new buttons and attach it here. Then if someone has time before the testing sessions and wants to build it in, they can.

yoroy’s picture

ry5n please do, maybe someone else can roll the patch based on your input then. Thanks!

ry5n’s picture

@yoroy I'll get on it ASAP.

Bojhan’s picture

Could anyone re roll the latest patch so it applies cleanly/no failed tests? We want to start applying the current one to the usability testing instances, so we can start preparing. (These instances can be updated, when new improvements are in before testing starts).

ry5n’s picture

RE: #119, I’m attaching some flat files that someone with more PHP and Drupal experience can roll into a working patch for the publish buttons. If the markup can be output as attached it should just be a matter of tacking on the css and linking in the js. Correction: linking in the js AND activating the button dropdown with this jQuery: $('.dropdown-toggle').dropdown();.

Note that I’ve thrown this together real fast (hence ported code from Twitter Bootstrap and overall messiness) but should suffice for testing.

ry5n’s picture

Oh, forgot to include a screenie. Should look like this:

xjm’s picture

@Bojhan, getting the patch to not fail tests will probably be a lot of work. I expect many of them codify the current user interface, and so to remove the failures we'd need to codify the new interface. I'd suggest doing the prototype UX testing first before going down that road.

lslinnet’s picture

This is some great work, seems like a step in the right direction.

I did have a few concerns about this:

  • Trying to submit a form which haven't filled out all it's required fields, takes you to the field that haven't been filled out correctly - which is really nice - but it needs to take into account if the admin bar is set, and then scroll a little further down than that.
  • The accordian seems alot slow when there is only very few elements in the tab (take the publishing options tab, as an example it unfolds very slowly).
  • How will it work with more complex content creation screens - article/page are in my eyes too basic to do any proper testing.

Further more it would be nice if it was possible to make on of the accordion tabs unfolded by default.

millaraj’s picture

Looking fantastic and already a great improvement over what is already a step forward in D7 from D6. Congratulations already to those involved.

In regards to the accordian, it might be helpful if we only open one section at a time. So when a new section is clicked, it opens, and the presently open one closes. Might make for less vertical scrolling in the long run.

I'd agree with #130 in that the test examples are perhaps to basic to test properly.

One area that perhaps needs some attention is the "Text Format" area. Whilst it's helpful to be able to switch between different types of input, is it necessary to have all the text associated with it displayed for each one? e.g.

  • Web page addresses and e-mail addresses turn into links automatically.
  • Allowed HTML tags: ...
  • Lines and paragraphs break automatically.

I'd say that perhaps the same goes for many of the fields. (image, etc). Perhaps they should be hidden behind some hover over popup to reduce the extraneous text on the screen that just confuses the eye? With the number of fields on display at the moment it's not really an issue, but with a large content type, it gets hard to see where the fields are and what's just extra instruction. If, for example, you had 5 different types of wysiwyg fields, you're essentially displaying the same information 5 times. As we move more towards mobile, perhaps this extra screen real estate isn't worth it?

mariomaric’s picture

This is looking very good!

I wanted to just propose follow up issue to resolve adding menu item in http://drupal.org/project/hierarchical_select style, or some other similar module that is trying to resolve this, IMHO, PITA issue..

dgastudio’s picture

as alternative for menu management...

http://drupal.org/project/menuux

mariomaric’s picture

@kervi: Menu Browser is my second choice - BUT, something like this should be in core and is definitely making content creation experience better.

rovo’s picture

Priority: Major » Normal
FileSize
134.43 KB

This may be completely unrelated to the task at hand, but after testing the demo( http://dev.test567.gotpantheon.com/ ), my initial impression was that I would have a better sense of what words I would Tag the entry with, after I entered the body of content. If it was a very long entry, I may even want to make tags while entering the content. In the later case, I imagine the tag entry being in the sidebar instead of above the Body text-area field.

Attached current iteration for illustration.
Drupal 8 Node Edit

cpliakas’s picture

Priority: Normal » Major

Setting priority back to major unless there is a driving reason to do otherwise.

nevets’s picture

I do not think tags belong in the sidebar, in general the sidebar holds administrative settings and is not seen by all users. I also like the D7 behavior that lets me place the taxonomy field where I think is appropriate on the form

rovo’s picture

Your right, it can easily be moved in the admin. I was mainly referencing the original image posted for the design proposal at top, wondering about the switch from the Taxonomy in the sidebar to Tags above the content area.

http://drupal.org/files/createcontent-redesign.jpg

lslinnet’s picture

Adding the ability to move fields widgets into the sidebar would be a major plus - as it happens that we decide to create administrative functionality as fields on a node.

What I am basically thinking is that we get a functionality like the nodeformcols module. (or at least a checkbox that allows you to define that it goes into the sidebar)

hmartens’s picture

Category: task » feature

uhmm...it would be SUPER if once you have uploaded your images, you can drag the images to where you want it in your post...that would be a super-awesome feature :)

THAT would make it easy for all drupal and non-drupal users to make their posts look great!

aspilicious’s picture

Category: feature » task

That is not the goal of this issue.
I know you all want to help but PLEASE keep issues in scope.

Thnx

Everett Zufelt’s picture

I took a look at the demo. A few comments from an accessibility perspective.

1. The vertical tabs (collapsed fieldsets?) come after the Save / Preview buttons, so they are very easy to miss as a screen-reader user.
2. There is a heading "Published" that seems to make no sense as a heading.

Bojhan’s picture

Thanks for the review, we are currently still usability testing (already did, 10 participants). But once we are done, and validated the concept works or not. I am sure we can tackle these accessibility issues, they appear not too difficult to solve.

Bojhan’s picture

We have almost finished our testing (9/10 participants), we are still looking for help testing/finding one more participant that isn't familiar with Drupal. Anyone can help out with this?

Bojhan’s picture

We have now tested all the participants, and we are working on the analysis. This is going to take shape this week, and we plan to release the findings this week!

As it currently looks, we will be updating the spec - but no further testing, which means that next week we can get rolling on improving the UI and moving to the fidelity of a full core patch!

ry5n’s picture

Yay!

A big thanks to everyone who helped/is helping with the testing. Looking forward to the findings and rolling some new patches.

Bojhan’s picture

Thanks all for being so patience in awaiting the research results, but they are in! In general the tests went really well, both beginners as intermediate users understood the purpose of the sidebar and were able to use it effectively. We found a lot of specific issues around the summary, text formats and specific functionality in the accordions - but few problems with the overall design.

We would like to actually move this from just a prototype patch, to an actual core patch that is committable.

Below are the changes we would like to make reflecting the usability testing we have done:

  1. Publish state selection near save
    A lot of participants still struggled knowing what “Save” would end up doing to their content. We could mark this as a high to critical issue in the testing, and probably a direct result of us not implementing the state dropdown near save, lets make sure we add that
  2. Visual separation between accordions was occasionally hard.
    We noticed as people went through opening the different accordion tabs one by one that they often left the previous one open, creating a visual effect where it was often hard to notice which fields belongs to which tab. This could be solved by throwing in a more distinct border area between tabs, a mockup for this is coming up.
  3. Error messages are often hidden from view (bug)
    It seems like there are a few bugs that were triggered during the testing - one of them was error messages. They are shown only half, and there were no visual indicators a field has an error.
  4. No apparent need for summaries below accordions
    We noticed no clear need for summaries below accordions in the testing we did. Although vertical tabs had this pattern to give people a clue on the settings, we never noticed in the testing we did on intermediates and beginners that people would actually pay attention to this summary. Therefore our current conclusion is that we do not need these summaries.
  5. Text format has too much visual prominence
    We noticed a small difference between earlier tests in that people were even more drawn to the text format settings than before. Perhaps this is due to form elements that we increased in size, again a mockup for this will be coming up.

We want to keep this issue focused on the actual redesign, so the specific problems that we found which where not necessarily related to this redesign we moved into separate issues.

#1562776: Edit Summary label is unclear to users
#1562782: Menu settings on content creation are unclear
#1562798: Its hard for users to provide valid input in the authoring date field
#1562802: Allow for more images in article
#1562804: "Sticky on top" is hard to understand

So lets try to address 1-5 in this issue, and go through the review process applicable for these kind of changes.

moshe weitzman’s picture

FYI, accordions can be configured so that they always close the old tab when a user clicks on a new tab so that user only sees one open section at a time.

Also, I'd be sad to see those Summaries go away.

pfrenssen’s picture

I would like to retain the summaries too, this is a major time saver. Imagine an experienced content editor that needs to verify some settings on dozens of nodes, and has to click open each accordion in each node to do so. I wouldn't remove features that are genuinely useful to power users just because it is not useful for beginners.

Everett Zufelt’s picture

+1 on summaries. They are useful from an accessibility perspective. being able to access the summary without opening the tab and navigating to the form elements is great.

RobW’s picture

I agree with the above three comments. As we try to make Drupal more user friendly for non-experts we should still remember that Drupal can power almost anything. The system needs options.

It seems like the solution is config to choose if the accordions allow one or more than one open at a time, display summaries only when provided, and don't provide one by default. A little work in some other project issue queues, but that's ok with an interlocking, extensible system.

Bojhan’s picture

I am not hung up on not putting in summaries, if that's something the community wants. However so far we have not found any convincing data that people actually use it, other than a few occasional moments. So we can put it in for now, but even then lets get some activity going on all the actionable parts!

@ry5n, karschsp, jide any of you can push this forward again?

ry5n’s picture

@Bojhan I’ve read through the results and sub-issues, just haven't had a chance to comment til now. I hope to have some time to post my thoughts later today, and to work on patches through the week.

ry5n’s picture

Title: Implement the new create content page design » Seems in sum:
  • Issue 3 is pretty straightforward: QA with some layout fixes;
  • Issues 2 and 5 should be easily solvable by tweaking visual hierarchy and contrast, but I guess we should wait for those mockups before dealing with that;
  • We'll put issue 4 on the backburner for now, though I’d rather see the summaries hidden by default pending further evidence from user studies;
  • Issue 1 bears a little more discussion:

The save/publish buttons have had several UI variants through the design process, and after reviewing things including the latest PSD and the screenshots on various threads I'm no longer sure which option if any has been chosen as the final design. The screenshot in the OP has the Save button as a split-button with 'Save Draft' as a dropdown option 'Save Draft', but the PSD and late in the design process the save widget is a normal button with an associated select element with publish options. Is one or the other of these preferred?

My inclination is towards the button + select combo since the publication state is immediately visible – new users will have a harder time missing it and probably an easier time manipulating it.

As an aside, the overall placement of the save buttons on this screen is still an open issue. My thoughts on that are over in that thread.

I'm planning on tackling issue 3 (those messages getting cut off) later this week.

Gábor Hojtsy’s picture

Title: Seems in sum: » Implement the new create content page design

Changing the *issue title* changes the main issue title itself. It is not your comment subject.

ry5n’s picture

Sorry about that folks. Issue title fixed.

ryan.armstrong’s picture

Just weighing in on the summary bit. I don't think we should remove the summary, as it is very helpful, and something that I have used on many sites, especially blog or resource sections. I just think we need to come up with a better term for it, and perhaps have some hint text on how to use it. I posted more detailed thoughts in the issue specific to it, I just wanted to throw my vote in here for not killing it, but making it better explained.

Edit: Link to my comment in more detail: More Detail

ry5n’s picture

New patch addressing #147 (applies against the current 8.x HEAD). Includes the following changes:

- Ensure system messages are visible for node/add and node/edit screens (#147, point 3);
- Hide breadcrumb (as per comp);
- Improve visual contrast within accordion, especially between adjacent expanded settings groups (#147, point 2). Cleaned up CSS in the process;
- Ensure layout adjustments for accordion only apply when accordion is present (had been causing slight weirdness on node revisions screen);

ry5n’s picture

Updated version of previous patch.

- Refactor accordion item styles to avoid them applying too broadly;
- Address bug in Chrome where shading artifacts may be drawn across the width and height of the viewport when select elements are animated into and out of view. Should be tested/applied throughout Core. See http://code.google.com/p/chromium/issues/detail?id=122035.

dgastudio’s picture

FileSize
89.87 KB

Hi everyone!

first of all, sorry for my english.

i have something to say:

all the redesign and rework of "new create content page" is really great and nice, but...

the proposed design separes CONTENT CREATION from CONTENT OPTIONS, right?

and it looks cool with default content type (title, body, tags)

but that about content type with a lot of fields???

take a look to my custom content type, done with the help of nodeformcols module (http://drupal.org/project/nodeformcols):
snap

how i'll do the same with proposed redesign?

so, i think, you must rethink the concept.

1. move all the visual improvements, to new admin theme, like RUBIK for example.
2. create new module for drupal 8, that ALLOWS to user to DISTRIBUTE content creation page elements AS HE WANT, not as YOU think he want. I mean, the same that NODEFORMCOLS but for d8.
3. create a default preset for this module, that distributes page elements as you think they must be.

thank you.

pfrenssen’s picture

@kervi, don't worry you will still be able to customize the node edit forms as much as you like. If you think about it, in D7 there is also a separation between the content and the options: the content was at the top of the page and the options at the bottom. This issue is about improving the default look and usability. If you are not happy with the default you can still change it in any way you like.

pfrenssen’s picture

Issue summary: View changes

added link to demo site (tvn)

Bojhan’s picture

Issue summary: View changes

Updated issue summary.

Bojhan’s picture

Issue tags: -prototype phase

Alright, I have updated the summary, lets keep working on this patch - there is no reason for it to stall, there is now data that confirms the approach works - all we need to do is fix the outstanding problems! Outstanding tasks are fixing the "publishing state" dropdown near save, accessibility bugs identified in #142 and code review. I will work on a design for text formats later this weekend.

Everett Zufelt’s picture

to add to my comment in #142, the Save button (etc) needs to be after the collapsed fieldsets in the DOM for screen-reader users, but tab focus still needs to be logical on the page. That is, when you tab through the page the visual focus needs to follow a logical flow.

ryan.armstrong’s picture

I wanted to throw out a quick idea. In some of the comps, we had the save/preview/delete buttons in the right sidebar at the top. I think it's a good idea to keep that there and add in some sticky functionality where it it is situated within the overlay but as the page scrolls up and it touches the top of the browser window, it "locks" to it and stays there until the user scrolls back up past it's original position within the overlay.

This would be similar to how the sticky tables work in Drupal. One thing that gets very annoying when editing content types either on a small screen, or with a lot of fields is constantly scrolling all the way down the page to hit save or preview. This would address that, as the actionable items would always be in the same position, upper right of the overlay (or page if overlay is disabled). Thoughts?

ry5n’s picture

I think it's an idea very much worth exploring. I posted a very similar idea here (it's one of the sub-issues for this one). Shall we discuss further there?

Wim Leers’s picture

I just finished reading through this entire issue. Great work everyone! :) Especially awesome work by @ry5n considering that it's his first core patch!

jide’s picture

While it is great to see this moving forward, I'm worried with the way this has been implemented. Could someone tell me if the current implementation is still considered as a prototype or is this more or less the way it will be really implemented ?

Having form elements lying in different parts (in sub arrays of $form I mean) in the node module do not sound like a good idea to me. I think it should either be the theme's choice or through some sort of subsystem that would enable custom layouts for forms.

Bojhan’s picture

@jide I agree with your concerns, I have asked jacine to take a look at this.

ry5n’s picture

Now that we're out of the prototyping phase I have similar concerns about the CSS. At the moment it's pretty ad-hoc, with a lot of pretty hacky selectors. I'd really like to modularize it, and also see how his fits into the bigger picture. That is, presumably many of these styles should live in Seven, and maybe we even need a style guide incorporating the aesthetic updates and the corresponding markup/style patterns. I'm sure @Jacine could provide direction on that as well.

ry5n’s picture

Yesterday I spent some time adding the publishing status control next to the save button. Having never worked with the Form API before, it was pretty easy :) [BTW thanks @Wim Leers for the shoutout.]

Results look OK, but to get the select element looking like the comp we'll have to decide to do more agressive form theming (again, I suppose this should be a decision made for Seven, not just this screen). I haven't really tackled the delete button. Finally, I did add a couple of lines to form_pre_render_fieldset() in core/includes/form.inc to make sure that collapsibles that are expanded when the page renders get the new 'expanded' class. Here's a preview of the patch at present:

create content patch 170

Jacine’s picture

Hey guys. First off, I'd like to say this is really awesome stuff.

Before I get into reviewing the actual code, there are 2 big concerns I have with this patch:

  1. It's soooo hard-coded. I completely agree with @jide in #167.
  2. While the desktop design is great, the mobile version needs to be done first, at least from a code standpoint. It would be really shitty to commit this patch without making mobile a priority and then leave it for the Mobile team to clean up.

Now, I realize that this is more a prototype than an actual core patch, and as a prototype it's fine, but reviewing it as a core patch will probably be helpful to move this forward, so here goes.

+++ b/core/includes/common.incundefined
@@ -6572,7 +6572,7 @@ function element_set_attributes(array &$element, array $map) {
-    if (isset($element[$property]) && !isset($element['#attributes'][$attribute])) {

It's not clear why this change is being made here, and frankly, I don't think it's related to this patch at all, so it should be removed. If there is a bug here, please create an issue for it (if one doesn't already exist).

+++ b/core/includes/form.incundefined
@@ -3673,6 +3673,8 @@ function form_pre_render_fieldset($element) {
+    } else {
+      $element['#attributes']['class'][] = 'expanded';

This class is not needed. Targeting .collapsible will work for the expanded view and then .collapsible.collapsed below it will have the same effect. It could be a useful helper class, but again, this should be done in this issue. (personally, I think data attributes should be used in an updated implementation, i.e. data-collapsible="expanded/collapsed").

+++ b/core/includes/form.incundefined
@@ -3673,6 +3673,8 @@ function form_pre_render_fieldset($element) {
@@ -4371,33 +4373,37 @@ function theme_form_element($variables) {

@@ -4371,33 +4373,37 @@ function theme_form_element($variables) {
   $prefix = isset($element['#field_prefix']) ? '<span class="field-prefix">' . $element['#field_prefix'] . '</span> ' : '';
   $suffix = isset($element['#field_suffix']) ? ' <span class="field-suffix">' . $element['#field_suffix'] . '</span>' : '';
 
+  $description = '';
+  if (!empty($element['#description'])) {
+    $attributes = array('class' => 'description');
+    if (!empty($element['#id'])) {
+      $attributes['id'] = $element['#id'] . '--description';
+    }
+    $description .= '<div' . drupal_attributes($attributes) . '>' . $element['#description'] . "</div>\n";
+  }

This doesn't belong here either.

+++ b/core/modules/field/field.api.phpundefined
+++ b/core/modules/field/field.api.phpundefined
@@ -743,7 +743,6 @@ function hook_field_widget_info() {

@@ -743,7 +743,6 @@ function hook_field_widget_info() {
     'text_textfield' => array(
       'label' => t('Text field'),
       'field types' => array('text'),
-      'settings' => array('size' => 60),

This doesn't belong in this patch either. Personally, I think it needs to be changed, 60 is way too wide, especially for mobile, but this should be a separate issue as it affects many other places in core and isn't directly related to this patch.

+++ b/core/modules/field/field.api.phpundefined
@@ -743,7 +743,6 @@ function hook_field_widget_info() {
diff --git a/core/modules/node/node.admin.css b/core/modules/node/node.admin.css

diff --git a/core/modules/node/node.admin.css b/core/modules/node/node.admin.css
index 5777b1f15963061355d755805a47e4abb1020815..54bd874c3d2441a156b07a16621092e312927b14 100644

index 5777b1f15963061355d755805a47e4abb1020815..54bd874c3d2441a156b07a16621092e312927b14 100644
--- a/core/modules/node/node.admin.css

None of this code belongs in node.admin.css. This implementation is for the Seven theme, so it belongs in the Seven theme. Of course, anything we commit here should be tested in Stark first and foremost, and if it is decided that Stark should use this design (the very basics of it) then whatever is needed for that, if anything, would go in node.admin.css.

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+ * Resets.
+ *
+ * @todo These resets should be added to Drupal's core CSS.
+ */
+html,
+button,
+input,
+select,
+textarea {
+  font-family: sans-serif;
+}
+legend {
+  display: block;
+  float: left;
+  width: 100%;
+  white-space: normal;
+  *margin-left: -7px;
+}
+
+/**
+ * Remedy an issue in Chrome where shading artifacts appear on select elements
+ * that are animated into and out of view.
+ * See http://code.google.com/p/chromium/issues/detail?id=122035
+ */
+select {
+  position: relative;
+}
+
+/**
+ * Shrink-wrap the clickable region of labels to the label text.
+ */
+label {
+  display: table;
+}
+
+/**
+ * Labels with explicit associations are clickable.
+ */
+label[for] {
+  cursor: pointer;
+}

This definitely does not belong in system.base.css. This purpose of this file is for the bare minimal styles that Drupal needs to function properly. None of this falls in that category, and belongs in the theme's CSS file. Arguably, one or two of these can be candidates for system.theme.css, such as label[for] and the select rules. However, this would need to be discussed, in a separate issue.

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+  border-left: 1px solid hsla(0, 0%, 0%, .1);

Please use rgba and include a fallback for IE8.

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+/**
+ * Gradients on .edit-secondary look odd because they are drawn over by the
+ * .collapsible borders. Fix that with pseudo-elements whose z-index we can
+ * control. We can't use :after because it might interfere with clearfix.
+ */
+.overlay .edit-secondary:before,
+.overlay .node-form > div:before {
+  bottom: 0;
+  content: "";
+  pointer-events: none; /* Allow clicks. */
+  position: absolute;
+  top: 0;

This is hacktasktic. I haven't actually applied the patch yet, so I'm not positive how/why it's being used, but we need definitely something cleaner here. :)

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+  background-image: -moz-linear-gradient(left, rgba(0, 0, 0, .05), transparent);
+  background-image: -ms-linear-gradient(left, rgba(0, 0, 0, .05), transparent);
+  background-image: -o-linear-gradient(left, rgba(0, 0, 0, .05), transparent);
+  background-image: -webkit-linear-gradient(left, rgba(0, 0, 0, .05), transparent);

The spec version needs to be added at the bottom here.

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+.edit-secondary .collapsed + .expanded,
+.edit-secondary #edit-revision-information + .expanded,
+.edit-secondary .expanded:first-child {
+  -moz-box-shadow: inset 0 2px .5em rgba(0, 0, 0, .1);
+  -webkit-box-shadow: inset 0 2px .5em rgba(0, 0, 0, .1);
+  box-shadow: inset 0 2px .5em rgba(0, 0, 0, .1);

The + selector isn't supported properly by IE8, so it's not going to work out here.

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+.edit-secondary .collapsible .fieldset-legend {
+  background: none !important; /* Override system. */
+  display: block;
+  left: 0;
+  margin: 0;
+  padding: 0 !important;  /* Override system. */
+  position: relative;
+  right: 0;

We shouldn't use !important here, because we don't really need it. If you decide you really want to use it in the Seven theme, it's okay, but definitely not in the module's CSS file.

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+  font-size: 14px;

Gotta use ems here, and only if it's in the Seven theme. Font sizing is not a good idea in module CSS.

+++ b/core/modules/node/node.admin.cssundefined
@@ -1,12 +1,543 @@
+/**
+ * Cross-browser Jank
+ */
+
+button.button,
+input[type="submit"].button {
+
+  /* Old FF */
+  &::-moz-focus-inner {
+    padding: 0;
+    border: 0;
+  }
+}

I'm assuming this SCSS was pasted in by accident. ;)

+++ b/core/modules/node/node.pages.incundefined
@@ -206,22 +220,32 @@ function node_form($form, &$form_state, Node $node) {
+  if (isset($node->title)) {
+    $published = (isset($node->status) && $node->status == 1) ? t('published') : t('not published');
+    $node_summary = '<div class="node-summary"><h3>' . $published . '</h3>';
+    $node_summary .= '<p><em>Last saved ' . format_date($node->changed) . '</em></p>';
+    $node_summary .= '<ul><li><strong>Author</strong> ' . $node->name . '</li>';
+    $node_summary .= '<li><strong>URL</strong> ' . drupal_get_path_alias('node/' . $node->nid) . '</li>';
+    $node_summary .= '</ul></div>';
+  }
+  else {
+    $published = t('not published');
+    $node_summary = '<div class="node-summary"><h3>' . $published . '</h3>';
+    $node_summary .= '<p><em>Not saved yet</em></p>';
+    $node_summary .= '<ul><li><strong>Author</strong> ' . $user->name . '</li>';
+    $node_summary .= '</ul></div>';

These should be individual form items (#type => 'item') to the extent possible, otherwise this is a an altering/theming nightmare. It should definitely not be thrown into an element #prefix.

=======

So, there is a lot above to address (sorry), and I didn't really scratch the surface, because the patch really needs to be rewritten in the Seven theme's CSS file, and when that happens it will change quite a bit, but hopefully this review is helpful. I think that it'd be a really good idea to work with the mobile team as soon as possible as well.

Also, can someone please clarify if this layout is going to be applied to Stark? Personally, I don't think it should, but if that's in the plan, what should that look like? What about Bartik? This needs to be considered ASAP, so the code can be written properly from the start.

JohnAlbin’s picture

Issue tags: +mobile

Fantastic to see this work progress past user testing! Super excited.

Thanks, Jacine, for pointing out the need for a mobile perspective on this. I'll get some of the Mobile Initiative peeps to jump in and help out. We can add in the necessary media queries and stuff to get this working on smaller screens. The patch needs a bunch of other clean-up too, so now's the time to get it all going. :-)

moshe weitzman’s picture

FYI, we already have an accordion in core thanks to jQuery UI. See http://drupalcode.org/project/drupal.git/blob/HEAD:/core/misc/ui/jquery....

ry5n’s picture

Thanks Jacine and John for your input. I’m keen to to start refactoring the css and moving it into Seven, but I want to be on the same page as others working on the theme for D8. I don’t see much major activity in the issues, but if anyone is aware of other work being done, please chime in.

Now would also be a good time to build out the Seven style guide, since this project (content creation screen) includes aesthetic updates that will apply across the theme.

yoroy’s picture

Great work all, we've certainly made big leaps forward here. Also, approaching 200 comments in an issue where we're only now gearing up for actual implementation of the design :)

Lets outline a battle plan for moving this into actual implementation, create/link the relevant issues, post that to the summary in here and consider this the meta-issue for tracking progress in general. From what I read in the latest comments:

- Clarify the mobile-first approach for this design
- Formulate an approach that tackles the concerns in #167
- Decide on what the Stark version of this should be
- Decide on what the Bartik version of this should be
- Review design decions made here for their system-wide impact on Seven theme
- I don't know if accordions, CSS refactoring, new regions and whatnot should be split out into multiple issues or not. Instinct says to go for a first big patch that puts most parts in place, focussing on sound architecture. Use follow-ups for more detailed work on individual page elements, but please do advise.

jide’s picture

About the implementation, what are your opinions ?

For now, I see 2 options :
- Leave the node module form as it is (I mean before this patch), and let theme decide how they implement the form.
- Use some subsystem, I'm thinking of something similar to the #group property used for vertical tabs.

Thoughts ? Other suggestions / ideas ?

ry5n’s picture

[Edit for egregious grammatical error :)]

Sounds good. @yoroy I think your instinct is right to keep the first big patch in this issue; I'll second that as an approach.

I feel like we should spend some time in some form discussing how this will impact Seven as a design system, but without an existing style guide I wonder if we could simply focus on clean front-end architecture, make the changes needed for the content creation screens (with styles changed globally) and see what breaks elsewhere. Then we can triage issues with something concrete. Does that make sense?

jide’s picture

Another proposal for the implementation, something more radical :

We could have a system to compose any form inside layouts, merging efforts with the Blocks & Layout initiative UI (http://groups.drupal.org/node/227543), blocks being form parts. Node form would simply be a predefined layout.

That would be a great step towards something constant (anything that is displayed in a Drupal site could be composed using the same approach).

What do you think ?

ry5n’s picture

@jide Even though I’ve been keeping tabs on Scotch and in particular that issue I still don’t have good grasp of the issues involved, especially the back-end architecture. Anyone else have implementation input, given the current state of Scotch and WSCCI?

webchick’s picture

Hey ry5n! I notice you're in Vancouver. I'm also in Vancouver (er, ish)! Are you coming to http://drupalcampvancouver.com/ this weekend? If so, maybe we could sit down for an hour or so and hash out answers to your questions? (And summarize here in the issue.)

ry5n’s picture

@webchick I ’ll be there! And I would love the chance to sit down with you and work through some of my questions. I really appreciate that.

jide’s picture

To quote @webchick's post at http://buytaert.net/spark-update-in-line-editing-in-drupal :

Later, we are hoping to branch out into other areas of authoring experience too, including helping with the content creation form improvements that the Drupal usability team has been spear-heading, as are well as the layouts UI work being actively discussed in the usability group.

This makes me feel that a unified approach for layouts, including form layouts, is the right direction.

jide’s picture

ry5n’s picture

At Drupal Camp Vancouver this weekend I had the chance to chat with webchick about this issue which was really great. (Thanks webchick!)

Based on that discussion, I don't think we should wait on any of the major initiatives – Scotch in particular – since most of them are not at the stage where we can benefit. Since the changes to core are confined to this one form and the associated CSS, I think we should implement the changes as designed using what's currently available. If we do it right it might need some changes later for Scotch etc, but shouldn't be an outlier that needs special treatment. The one exception is the mobile initiative – we should make sure our implementation jives with that right away.

Assuming that makes sense, @jide: would you like to team up to work on this? Right now I'm in the early stages of integrating CSS changes into Seven.

cweagans’s picture

I hate to throw a wrench in things, but would it be worth it to use this design for all of the new entity pages (for instance, the new taxonomy term page)? I know that they aren't really the content that this design was built for, but they are content in the site.

Or perhaps using this layout should be controlled by some flag in the form? That way, the entity pages that should use it can, and the entities that shouldn't can just use what we have right now?

jide’s picture

@ry5n: I understand your point, and obviously waiting for other initiatives would hold the issue for a long time.

The only thing I'm afraid of is to get to a point where Scotch would eventually enable custom form layouts and a node form implemented in a different way which would lead to re-implement it, or leave it as is.

But :
- I think they did not think about form layouts yet at the Scotch initiative, we should ask them anyway.
- It may even not be feasible or a good thing (I think it would be !).
- In the hypothesis Scotch would allow layouts for forms, implementation could be straightforward enough to allow us to keep the CSS and markup structure if we did it now (or small / easy changes), so beginning to work on it now would not be lost time.

Bottom line, I agree, let's implement this at the theme level for now, we'll see where Scotch goes, but I personally would be in favor of doing as less ad-hoc things as possible in the perspective of keeping an eventual Scotch for forms in sight.

Separating layout and styling would be a good practice if we want to be able to re-factor things at some point.

I also agree with @cweagans, and that's why I did mention #1499596: Introduce a basic entity form controller which aims at having a generic form for all entities, although a global switch would introduce some "layout forms" specific APÏ, which I'm not too much in favor of.

Anyway, I'd be glad to help.

JohnAlbin’s picture

would it be worth it to use this design for all of the new entity pages

It's very likely, yes.

I think jide is correct in his direction, BUT the SCOTCH initiative is not even close to having usable layouts available to use.

Separating layout and styling would be a good practice if we want to be able to re-factor things at some point.

Agreed! Not only do we need to separate the layout from the rest of the styling for a possible SCOTCH refactoring, we also need to separate it because we want it to be reusable for other forms. There's nothing "create content"-specific about this layout.

So there are a few levels of separation we need to address in this patch:

  • layout — I'm going to use "layout-2-col" as the machine name here; but only because I spent zero time thinking of a name. Suggestions welcome.
    • layout-2-col.css: contains basic layout styling with CSS3 media queries for device/mobile responsiveness.
    • layout-2-col.tpl.php: markup needed to support the CSS layout
    • layout-2-col.theme.css: General CSS needed to make the layout usable (in Stark).

    These things would reside in the system module until SCOTCH's layout stuff is ready.

  • system.base.css: theme-independent form styling needed for the form to work.
  • system.theme.css: general CSS needed to make the form usable (in Stark).
  • node.admin.css: Specific CSS needed to make this form usable (in Stark). NOTE: hopefully this isn't needed. It would be preferable if form CSS was tied to the widgets used and not to this specific module's form.
  • seven/style.css: CSS needed to get the form usable and pretty in Seven.

Right now everything is in node.admin.css.

jide’s picture

Yes agreed, whether Scotch will allow form layouts or not, separation of layout and styling will be beneficial and allow reusability.

I like John's proposal for the template / CSS structure, it makes sense and is clean. But shouldn't we have a CSS tied to the generic entity form instead of using system.XXX.css ?

I'm not even sure we should style anything at core's level, shouldn't we do this at theme's level instead ?

The layout-2-col.tpl.php template file would be a suggestion added through a preprocess function ?

Again, is there a reason we don't leave the node form intact in core and use the theme to add template suggestions and specific CSS ? Or was it the idea ?

jide’s picture

What I suggest :

  • Core level :
    • We have some general form styling changes here I think, things that are not node / entity specific, this should go in system.theme.css anyway.
  • Theme level :
    • Add the layout logic.
    • Style specific things, in particular elements in the right side.

That would lead to add the layout logic in each theme. So, what do you think ? Does the layout thing belong to each theme, so we could imagine some themes may want to have a different layout, or is 2 cols layout what core expects and it should be implemented at core's level ?

RobW’s picture

I'm with jide; the layout needs to be at the theme level. 2-col is also probably not the best naming convention, seeing as how the mobile/small screen version won't be two columns.

To me it seems that the cross theme functionality prototyped in this issue is the user-configurable separation of content and settings/meta fields, the collapsing behavior on the settings section, and the publish state submit button grouping. Beyond that, things should probably be left as a blank slate for theme developers. Stark should definitely have the 2 column large screen layout (and whatever's decided on in mobile) with plain unstyled form elements, and serve as a model for other themes. Seven can duplicate the layout CSS (or use Stark as a parent theme, ooh maybe I'm getting carried away here), and include the visual style. But at the system level we should strive for as little design decisions and css as possible, and let interested developers write new admin themes without having to override or replace system CSS.

JohnAlbin’s picture

But shouldn't we have a CSS tied to the generic entity form instead of using system.XXX.css ?

And which module is responsible for the generic entity form? It's system, isn't it? Our CSS is organized by the module that adds it to Drupal.

I'm with jide; the layout needs to be at the theme level.

You can't put the logic for adding the 2 column layout into the Seven theme, because that means ALL they CSS goes in seven/style.css. And that makes refactoring for SCOTCH much harder. SCOTCH isn't going to add layouts to themes. It's going to add layouts at the module level.

The separation I outlined allows SCOTCH to just grab the layout files out of system module. The separation also allows any theme to use a generic-looking 2 column layout (for example, our other 2 core themes, Bartik and Garland). And if the media queries in the layout don't work for a particular theme, they can override the stylesheet and pick better ones. Otherwise, every single theme has to re-implement the same code to get the benefits of this design.

BTW, I thought about the machine name a bit and came up with “layout-meta-sidebar”. Thoughts?

jide’s picture

I get your point John, but it is valid only if "form-enabled-SCOTCH" goes somewhere in the end. Otherwise, we would have layouts at module's level and no good reason for it IMHO.

To sum up :
- If SCOTCH will allow layouts for forms, we should anticipate and do layout at module level in the meanwhile, in a clearly separated part that will be easy to put out after SCOTCH lands.
- If it won't, we should implement it at theme's level.

Do you agree ?

So... I guess it's time to ask SCOTCH team what they think, what I'm going to do now :)

jide’s picture

I've posted http://groups.drupal.org/node/235738, let's see what happens now.

jide’s picture

Whoops ! what's with the tags ??

Feel free to support the idea on the group post, the more we are, the best chances are this happens.

David_Rothstein’s picture

In general, basic design decisions belong at the module level, and we already have great guidelines at http://drupal.org/node/1089868 that explain how to do that while still allowing particular themes to swap out the design completely if they choose to.

So why can't we just use that method rather than inventing a new policy?

It probably is the case that there's a gray area between module.base.css and module.theme.css that layouts could fall into, and therefore some rationale for a new kind of layout-only CSS file. But it's still not totally clear why this issue needs to be the one to introduce that.

As for which module this goes in, if we're talking about entity forms perhaps entity.module makes sense... although I can see the rationale for system.module too (if this is a generic layout that other non-entity forms might want to use as well).

jide’s picture

@David, the guidelines are a useful resource, however while "basic design decisions" belong to modules, I'm not convinced that a node form layout with 2 columns is a basic design decision, but rather a strong one (a good one to me in many cases anyway !).

SCOTCH apart, to me a basic form as we are used to is the basic design decision and belongs to module level, while "layouting" the form is a stronger point of view and belongs to the theme. To me, it would be as legitimate to have 3 columns with some meta at the left and some other info on the right. Or 2 columns on the right, why not (you get the point).

I do not see how we could make a 2 columns layout form at the module level without introducing some layout specifics in the code, while I totally see this being done by the theme using hook_form_alter(). Yes maybe we'll have to do this for Seven and Bartik, but that's not such a big deal to me, and maybe leave Stark alone with the default form.

That said, all of this would be different if SCOTCH initiative will allow form layouts. In this case, it will neither the node/entity/system module nor the theme which will be responsible of the layout, that's why we considered having a temporary solution in this case.

ry5n’s picture

I'm going to have less time to work on this issue over the next three or four weeks. I'm taking Stanford's online HCI course, and it's requiring more of my time than I hoped. That doesn't mean I won't be here (I most certainly will), but I may not have a lot of time to write code until early July. ;_;

webchick’s picture

No problem, ry5n! Thanks for keeping us in the loop.

Here's a re-roll to the current state of the 8.x branch, at least code-wise. I'm not sure what to do about these other failures:

error: missing binary patch data for 'core/misc/menu-collapsed-rtl.png'
error: binary patch does not apply to 'core/misc/menu-collapsed-rtl.png'
error: core/misc/menu-collapsed-rtl.png: patch does not apply
error: missing binary patch data for 'core/misc/menu-collapsed.png'
error: binary patch does not apply to 'core/misc/menu-collapsed.png'
error: core/misc/menu-collapsed.png: patch does not apply
error: missing binary patch data for 'core/misc/menu-expanded.png'
error: binary patch does not apply to 'core/misc/menu-expanded.png'
error: core/misc/menu-expanded.png: patch does not apply
Bojhan’s picture

Just an update, for this patch in terms of Seven styling. I think we want to do this in a few steps, so the individual form styling should not be part of this patch. The buttons and sidebar obviously are.

I am not sure what people feel its held up on, we want to apply this to the Seven theme as much as possible (almost nothing in system.css files if possible). Stark should remain the same.

fuby is workin on this, atm.

Bojhan’s picture

Issue summary: View changes

ey

Bojhan’s picture

Issue summary: View changes

Updated the todo's

fubhy’s picture

Status: Needs work » Needs review
FileSize
29.32 KB

Did some code style fixes. Also, there were a lot of missing t()'s. I slightly changed the form and reduced the amount of markup for the info block. I also did the CSS for the form actions area and the delete button as well as some other minor changes.

Note: I can't test it on any other browsers than Chrome right now so it might look broken on IE or Firefox.

There are some things that I noticed while working on this patch which I didn't fix yet (not enough time :) ).

@todos:
- Add CSS3 Transition when showing / hiding the tabs (if possible).
- Extract the new fancy CSS from the node.admin.css to the theme layer. Everything that is not part of the basic content creation CSS should be in the theme imo.
- The published/not published select dropdown (the one next to the submit button) should be styled so that it looks more button-ish (as seen on the graphics).

fubhy’s picture

Oops... My IDE doesn't like to edit CSS it seems. Also fixed a wrong margin / padding in the node summary pane.

Status: Needs review » Needs work

The last submitted patch, node--create-content-1510532-201.patch, failed testing.

ry5n’s picture

Go @fubhy! BTW don't miss @jacine's code review further back in the thread. There's a lot of refactoring to do. I really really want to work on this but as I mentioned above I just don't have the time at the moment.

jide’s picture

@Bojhan, @fubhy, @ry5n: Should we create a separate issue for Seven theme styles that are not tied to the content creation page ?

Bojhan’s picture

@jide Yes! I am not sure about any of the specific form styling in this patch. We need to see how it looks on other pages.

From my point of view, we can separate out the buttons or keep it here - whatever works best.

ry5n’s picture

@Bojhan @jide @fubhy Although my time is still limited, I created that issue to track wider theme changes to Seven. Bojhan: please change anything about it as needed.

fubhy’s picture

I am currently on holidays in spain (staying here for a couple of days the dev days). However, I will provide a second patch today or tomorrow once my girlfriend allows me to hack for a few hours :). I will include jacines review in this one and start splitting up the css and moving it to the files in which it actually belongs. That said, the upcoming patch is probably going to be an entirely refactored version of the original code.

David_Rothstein’s picture

I am not sure what people feel its held up on, we want to apply this to the Seven theme as much as possible (almost nothing in system.css files if possible). Stark should remain the same.

If this is only applied to the Seven theme, then what happens to all other Drupal themes? We are going to keep the old experience (vertical tabs) for everyone else, but Seven will have a different experience?

I still don't understand how this will work if the default content creation experience in Seven is going to be radically different from the default Drupal content creation experience.

I do not see how we could make a 2 columns layout form at the module level without introducing some layout specifics in the code, while I totally see this being done by the theme using hook_form_alter().

We could do it the same way we currently do it for vertical tabs. The module would be responsible for putting its form elements into two different groups (basically like they do now for vertical tabs), and the default theming would render those groups as two different columns. But a theme could easily swap out the two-column styling for something else.

RobW’s picture

+1 for #208. I was going to write up something similar, but you beat me to it.

Bojhan’s picture

@David I think you are right, we do want to allow other themes to have this with little to no effort.

fubhy’s picture

Assigned: Unassigned » fubhy

Yes.. That is exactly what I wanted to go for. The question is whether we want to provide this as an actual UI component like the vertical tabs so it can be reused anywhere. I would +1 that ... However, if we do that, all the styling that comes with that by default should be really minimal as suggested by you guys. All the advanced, custom styling should go into Seven.

I am assigning this to me for now.

jide’s picture

That's the other proposal I suggested in #176, and although I still have doubts on where this belongs (theme or module), David makes some good points.

But do you guys agree to do this in a simple/minimalist way for now until we know if SCOTCH will allow form layouts ?

Sorry if I look like I'm insisting, but I feel that we have a specific use case of something that could be reused and generic, and if we want to do this well, #groups won't be enough in the long term and we would miss an opportunity for the future IMO.

ry5n’s picture

@jide I say we do that: keep it minimal, usable by other themes, and not do anything that we know will be hard later. But we still don't know much about how SCOTCH will implement things, so we have to do our best.

Does #group work as a general grouping mechanism? When I was thinking through how to do this, one of my questions was whether to use #group or nested fieldsets.

cweagans’s picture

May I suggest actually talking to the SCOTCH guys? I'm sure that they'd be more than willing to point you in a direction that won't turn this issue into a gigantic CF in the future when the scotch work is committed to core.

#drupal-scotch

ry5n’s picture

@cweagans I've tried in a number of places, maybe not the right ones; haven't been able to get much in terms of concrete advice. Am going to ask EclipseGC directly.

EclipseGc’s picture

So, after a conversation with ry5n and webchick in irc, my suggestion would be that this can be done fairly simply for the moment. The layout is less concerning to me than the components within it. You have the left content, right content, and then the buttons (whose placement I think you're still arguing over). What's important is that these 3 separate sections be really well thought out, dependency injected with appropriate information, and separated cleanly. From there, you can probably just put a theme wrapper around it. Something like:

http://pastebin.com/JV7yHDSF

If this isn't clear enough let me know. I would personally do a LOT more componentization, but I think that's more my problem than it is yours, so this is probably the bare minimum you need to worry about. As far as the layout itself, that's a separate concern, and I think as long as we conform to something that's pretty typically drupal, I'll have a clear path to migrate it to scotch.

Eclipse

David_Rothstein’s picture

Sounds like everyone is mostly on the same page then.

Regarding #group vs. $form['left'] and $form['right'], one thing to keep in mind is that #group is designed to be much easier to alter. If you want to move something in or out of vertical tabs, that's easy to do right now just by altering the #group in hook_form_alter(). Whereas if you have to physically move the array from $form['left'] to $form['right'], that will really complicate things for anyone else who is altering the form (because now they need to look for it in both possible places), plus it could mess up forms with a #tree structure, etc.

I don't think it's necessarily a showstopper, because there are other ways around this (e.g., you can move form elements around in a #pre_render callback rather than using hook_form_alter), but it's definitely something to keep in mind if we want people to be able to easily alter this via standard hook_form_alter() methods.

I imagine if you read through #323112: Vertical Tabs you can get more details on exactly why vertical tabs were implemented the way they were... I didn't take the time to read through it comprehensively myself, but I think the above is the gist of it :)

webchick’s picture

Just including the pastebin from #216 in-line here since those things have a way of disappearing:

function node_form($form, $form_state, $type) {
  $form['#theme_wrapper'][] = 'node_form_layout';
  $form['left'] = node_left_form($form, $form_state, $type);
  $form['right'] = node_right_form($form, $form_state, $type);
  $form['actions'] = node_actions_form($form, $form_state, $type);
  return $form;
}
jide’s picture

I second David here, that's the implementation I was worried about, whatever happens next this will introduce bad things IMO. I'm strongly in favor of not changing the $form structure.

@cweagans Thanks for the IRC chan, good to know :)

EclipseGc’s picture

Actually the bigger problem here is that we should be calling


$form['left'] = drupal_get_form('node_left_form', $type);

But we can't do that because we need to pass the formstate along to node_left_form() because the formstate gets reset by drupal_get_form (which is a serious serious problem in my opinion, but I don't have the bandwidth to go change it). An alternative to this is to allow the configuration of the node array that is typically passed through formstate by functions like node_add to be passed through something that is NOT the formstate (i.e. more dependency injection). In this scenario no one would form alter the wrapper form, but would form alter the sub forms for the case you cite, which is actually the better solution and more more in line with scotch's end goals. The real problem here is that beyond this left/right paradigm you have fields and extras and buttons and stuff that all need the ability to be rendered in isolation from the rest of the form, and the other components with thing the form need to be smart enough to not re-render stuff that's already accounted for in other components. But I really don't feel like this thread needs to worry about that stuff since a lot of it is my problem and not yours... Still it's worth mentioning so that people know what's bouncing around in my brain when I see this. I didn't want to complicate your implementation which is why I kept it ridiculously simple, but I think the short solution is that $type needs to be an array that would normally be used as the pre-node array that is built by stuff like node_add() and not just a simple string stating 'page' or 'article'. This allows us to use drupal_get_form() instead of calling the form functions directly, and that allows us to do staged alterations as necessary.

Eclipse

Bojhan’s picture

@fubhy Are you still working on a patch? Otherwise unassign.

sun’s picture

Early feedback from (AFAIK) the most experienced person in this field: http://groups.drupal.org/node/235738#comment-775758

Gaelan’s picture

What's with the redesigned Toolbar in the mockup? I like it, but I was wondering were it was from?

fubhy’s picture

Assigned: fubhy » Unassigned

I will be back home on the 28th and will provide a patch as soon as I get a stable internet connection.

ry5n’s picture

EclipseGc++
sun++
swentel++
Thanks all!

@fubhy: looking forward to seeing that new patch, and looking forward to joining in again as soon as I can.

chx’s picture

Priority: Major » Critical

I can't see us releasing without this.

ry5n’s picture

[edit: adding link to swentel's suggestions; edit 2: adding patch code to help with review]

I'm back on this now (and will be until it’s done)! Roughed this out starting from scratch based on swentel's suggestions, the idea being to marge back work from the prototype later. This patch makes the following changes/additions:

//--- a/core/modules/node/node.module
//+++ b/core/modules/node/node.module
@@ -174,6 +174,11 @@ function node_theme() {
     'node_recent_content' => array(
       'variables' => array('node' => NULL),
     ),
+    'layout_form_3' => array(
+      'render element' => 'form',
+      'path' => 'core/modules/system/layouts',
+      'template' => 'layout-form-3',
+    ),
   );
 }

+ /**
+ * Process variables for layout-form-3.tpl.php
+ */
+function template_preprocess_layout_form_3(&$variables) {
+  $form = $variables['form'];
+
+  // Build rendered layout parts for template
+  $variables['layout'] = array(
+    'secondary' => drupal_render($form['additional_settings']),
+    'actions' => drupal_render($form['actions']),
+    'main' => drupal_render_children($form),
+  );
+}
//--- a/core/modules/node/node.pages.inc
//+++ b/core/modules/node/node.pages.inc
@@ -152,6 +152,9 @@ function node_form($form, &$form_state, Node $node) {
     unset($node->in_preview);
   }
 
+  // Use a three-region form layout by default.
+  $form['#theme'] = array('layout_form_3');
+

@@ -209,7 +215,7 @@ function node_form($form, &$form_state, Node $node) {
   }
 
   $form['additional_settings'] = array(
-    '#type' => 'vertical_tabs',
+    '#type' => 'fieldset',
     '#weight' => 99,
   );
new file
+++ b/core/modules/system/layouts/layout-form-3.tpl.php
@@ -0,0 +1,26 @@
+<?php
+
+/**
+ * @file
+ * Form template with 3 main regions.
+ */
+?>
+<div class="layout-form-3 clearfix">
+	<?php if ($layout['main']): ?>
+	<div class="form-3-main">
+		<?php print $layout['main'] ?>
+	</div>
+	<?php endif; ?>
+
+	<?php if ($layout['secondary']): ?>
+	<div class="form-3-secondary">
+		<?php print $layout['secondary'] ?>
+	</div>
+	<?php endif; ?>
+
+	<?php if ($layout['actions']): ?>
+	<div class="actions form-3-actions">
+		<?php print $layout['actions'] ?>
+	</div>
+	<?php endif; ?>
+</div>

In a very narrow sense this achieves the basics but I’m sure it's not right. It needs review to determine:

  1. if there is consensus on this general approach, and
  2. what the layout preprocess function should actually be doing.

Let's save the template name until no. 1 is solved.

rovo’s picture

Status: Needs work » Needs review

What is the vision for how this will integrate with Drupal 8 and compliment the new Spark initiative of in-place editing? Will this new create content page be mainly used for the initial creation of a page, but then the in-line editing will be used only for pages already created.

This probably seems obvious, but not really clear to me. Will they come together in some way?

RobW’s picture

In line editing will be separate from the create/edit content page. Inline can only edit single fields in place, while back/admin-end editing is for an entire piece of content at once.

swentel’s picture

@ry5n I like the idea - did a quick review and that's what I've been thinking of yes, nicely done! Check the template file though, there are tabs in there instead of spaces. Let's hope some other people get eyes on this technique as well.

ry5n’s picture

@swentel Okay, great! @EclipseGc, if you have any feedback on this, please let us know.

I’ve fixed the tab indents in the tpl; will include that in a later patch. A couple of other questions:

  1. Curious: why is a form_alter a bad idea for moving things around?
  2. What is the reason for doing the rendering in preprocess rather than in the template itself? Layout re-use?

And a question for anyone: with the additional_settings element now a regular fieldset rather than vertical tabs, can collapsible fieldsets still use #group to add themselves there? It seems to work, but I can’t find docs that say it should. I'm concerned this is not intended usage and could break.

swentel’s picture

Curious: why is a form_alter a bad idea for moving things around?

Changing the internal structure of the form can break validate or submit functions as it's possible that the values of field in the $form_state['values'] array are not in the right spot. It's also logical to me that the form structure should not represent or expose any kind of logic how or where fields are nested, whether it's a simple wrapper or a fieldset.

What is the reason for doing the rendering in preprocess rather than in the template itself? Layout re-use?

No real big reason. Instead of already rendering them in the preprocess function, we could just assign them and then call render() in the tpl file. That's more the D7 way, but that's probably going to change anyway. Layout reuse is indeed one thing. Also, if a variable is empty, wrappers will not be printed empty in this case, which sometimes happens in D7, especially in the page tpl.

RobW’s picture

You should definitely call render in the tpl and not before. Otherwise you're serving non-alterable chunks of markup at the theme level, which is the exact problem renderable arrays were made to fix. Core needs to always err on the side of maximum configurability/customizability.

ry5n’s picture

Assigned: Unassigned » ry5n

@swentel, @RobW: thanks guys. I worked on this yesterday; more later in the week.

So far I've changed the preprocess function so it copies the sidebar and actions elements into their own template variables and unsets them from the $form. Rendering in the template is clean and simple.

The other way to keep the template clean while not rendering in preprocess sounds exactly like what @swentel suggested: a notation that maps render elements by name to regions in a layout definition. But I'm scratching my head a bit on that. Would it not end up being its own microsyntax, complete with a new drupal_render_layout function for use in templates? Please chime in if you have any insights.

This is what I'm planning next:

  1. Change node_form() in node.pages.inc to reflect the new design (adding/moving elements); refer to prototype implementation and Jacine's code review;
  2. See what this looks like in Stark; add core css if necessary;
  3. Start theming in Seven; again refer to prototype and ensure design responds to small screens.
David_Rothstein’s picture

Priority: Critical » Major

This is an important issue, but it shouldn't be labeled "critical".

As far as I know, "critical" would mean the current content creation screen is unusable (and people are getting completely stuck trying to figure out how to use it). But that's not the situation we're in.

ry5n’s picture

Priority: Major » Critical
Status: Needs review » Needs work

Still working on this, now on stage of theming for Seven. Hope to have a patch this week.

ry5n’s picture

Priority: Critical » Major

Fix priority. Didn't mean to change.

ry5n’s picture

I've been working steadily on this and have working code with most of the features complete, including initial testing across browsers and for mobile. However there is still quite a bit of work to do. Screenshots of present state are attached.

Of note:

  • I've been thinking a lot about an ideal mobile design for this, but will need to post that in a separate issue. Current code simply gets the basics (like source order) right.
  • Some media queries need changing so things still work nicely in IE8
  • The reset has been replaced with normalize as per Tame seven's reset css
  • This patch would close Seven theme form buttons have no hover state
  • There are a few things open to debate. Examples:
    • I've retained the breadcrumbs in a header bar, which also provides a nice home for error messages.
    • I've kept the collapsible fieldset triangle markers ("twisties") on the left of collapsible fieldsets, since they should be consistent with the rest of the theme
    • The font has been changed from Lucida Sans to Helvetica, as per the comp
    • Fieldset summaries are hidden by default, but can be shown by adding a class (.show-collapsible-summaries) to the body. This could be a preference.

Feedback on the visuals is most welcome as I get this code into a state ready for review.

dodorama’s picture

This looks gorgeous!
Still, I have two remarks.

1. While I love Helvetica, I believe its readabilty isn't excellent particularly at small font sizes. So I'll probably won't use it for descriptions and any text longer than a heading.

2. The sidebar, particularly when not in the overlay, is styled very similar to the input forms (text fields, textareas). It almost looks like and editable field.

Everett Zufelt’s picture

Curious if feedback from 142/163 has been incorporated and how the button positioning was solved.

Bojhan’s picture

@ry5n I would also be interested if we addressed the accessibility concerns of Everett in #240. In terms of the visual decisions, I would recommend placing the error and breadcrumb in the left side rather than creating a "bar" on top in which its placed. It creates this strange 3 layer pattern of regions. While we purposely designed for only two.

I don't think we should change the font-family in this issue, as that carriers a lot more review across Seven. The rest looks mighty good to me, we will want to fix the buttons - but this could also be done separately (whatever you feel like doing).

ry5n’s picture

@dodorama I agree in general about Helvetica as a UI typeface. Subjectively, it does start to be an issue at caption sizes. I actually like how it looks here, but if it were up to me I'd use PT Sans (with PT Sans Caption for small sizes) and something like Open Sans Light for main headings. @Bojhan I'll switch back to Lucida for the initial patch.

About the styling of the settings sidebar, I think you have a point. I'll see what I can do to differentiate them.

@Everett Zufelt I got the source/tab order fixed for the buttons by using three layout regions: the main fields and the form actions are floated left while the settings region is floated right. However, there are still a few minor tab-order issues I need to fix. I missed the problem with that 'Published' heading, so I'll be sure to fix that. Actually, you might be able to help. The section in question is the 'setting summary'. I want to have a screenreader-only fieldset legend for this section, but setting #title_display to 'invisible' on the #title doesn't apply the correct hiding classes. This sounds like something I should open a separate issue for, but if you know of a fix please let me know here.

@Bojhan I will also get rid of the header and make the buttons better match the comp as well. I have my own perspective on those but it can wait.

ry5n’s picture

Status: Needs work » Needs review
FileSize
74.54 KB

Finally, a new patch! Tested in IE8, 9, FF, Chrome, Opera. Almost there, but I need feedback and assistance with two remaining issues:

Issue: remove the 'header' bar

Not sure how to do this with the current layout implementation. As-designed, the following things would need to be moved into the left-hand column:

  • breadcrumbs
  • help block
  • secondary tabs
  • messages (console)
  • node preview

Can't be done with css or with any alter methods (most of those elements in page tpl). Possible solutions include a) scrapping the layout at the form level and re-doing it at the page level or b) consider allowing the header bar to stay. Requesting input!

Issue: form does not redirect properly on successful submission

Steps to reproduce:

  1. View a node page
  2. Edit the node (e.g. from a contextual link)
  3. Save the node

We should be redirected to the node again with a success message. Instead, we get the success message for a split second in an otherwise-blank overlay, and are then redirected to the node, with no message. The form action is successful though. I need help to solve this; not sure if the cause is even this patch.

Todo: finish RTL stylesheet updates

Some RTL styling is done, but need to complete and test. Would like to solve the above issues first.

Status: Needs review » Needs work

The last submitted patch, 1510532-no243.patch, failed testing.

ry5n’s picture

Status: Needs work » Needs review
FileSize
77.45 KB

Re-rolled against latest 8.x with flags for binaries (images).

aspilicious’s picture

Without overlay the right side is not in the correct spot. The redirect problems is a core problem not related to this patch.
This changed the entire button styling in the seven theme (just as a note to others)

Status: Needs review » Needs work

The last submitted patch, create-content-1510532-245.patch, failed testing.

andypost’s picture

+++ b/core/modules/node/node.pages.inc
@@ -312,31 +377,51 @@ function node_form($form, &$form_state, Node $node) {
+  $form['actions']['save']['status'] = array(
+    '#type' => 'select',
+    '#options' => array(
+      0 => t('Not published'),
+      1 => t('Published'),
+    ),
+    '#default_value' => $node->status,
+    '#weight' => 10,
+  );
+

this scares me, I think this "default" form will be very hard to form_alter

ry5n’s picture

@aspilicious Thanks for testing this. I've re-tested in Chrome, Opera, FF (all latest), IE9 and IE8 and positioning of the right sidebar seems correct and consistent. I did fix some aesthetic issues in non-overlay mode (patch attached). Note that there are media queries involved that will stack the layout below 900px, but if you're still seeing broken layout please post browser/version (and screenshot if possible).

This patch does make theme-wide changes (no sense having custom buttons for only this form). Note that I have taken a few small liberties with the design for contrast (accessibility), delete button styling (visibility) and have not e.g. implemented the custom select menus (overhead, maintenance, and honestly, lack of time). Other css changes end up applying theme-wide but any visual differences should be improvements only. If you find something that looks wrong, please post.

@andypost + core devs and themers: I'd be grateful for input on how to make this as easy to form_alter as possible. Feedback (and follow-up patches) are welcome. I'm honestly not the best person to polish the php, it's not my strong suit.

I'd love to hear from @Bojhan, @swentel and @johnAlbin on this, especially the header thing. If the design direction is indeed firm, I'd like to find a way to fix it.

ry5n’s picture

Status: Needs work » Needs review
FileSize
77.51 KB

Re-roll with fix for extra margin-top on accordion.

Status: Needs review » Needs work

The last submitted patch, create-content-1510532-250.patch, failed testing.

Bojhan’s picture

@ry5n Can you provide a screen?

aspilicious’s picture

http://www.diigo.com/item/image/1fbht/3q14 (without overlay)
http://www.diigo.com/item/image/1fbht/c6iq (with overlay)

I'm working on a full hd screen

Bojhan’s picture

I would love if we can tackle, the issue I mentioned in #241 because I feel that the breadcrumb bar still creates a lot of visual clutter. We can just wrap the breadcrumbs. The "Not published" should be bold. I don't think its about design decision being firm, it actually looks better - because instead of three regions we can get away with only two making it appear more simplistic.

The without overlay style should probably see removal of the "box", instead it should be a bar that runs from the top to the bottom of the page - with that being closer to the overlay styling.

I think the Seven maintainer should give feedback on this, but we should probably keep all the form style changes out of this patch (input elements, buttons etc.) - because it creates some weird inconsistencies on other pages.

ry5n’s picture

@Bojhan, it's more than just breadcrumbs that need to be moved: also messages/console area (form success/error messages), system help block, secondary tabs (even though not visible now) and the node preview content (screenshot with success message and node preview). Best way to do that is a new page template, I think. That's why I'm asking for implementation input.

Additional screenshots attached.
New create content page, widescreen layout, overlay mode

ry5n’s picture

Assigned: ry5n » Unassigned

I need to take a break from this for a few weeks for work backlog. Un-assigning until end of August though I'll be keeping close tabs. Til then, if anyone wants to take a crack at the last push, please do! I'm also in #drupal-contribute on-and-off if anyone has questions.

kika’s picture

Currently you can apply latest patch using

git reset 9532b4b --hard
curl http://drupal.org/files/create-content-1510532-250.patch | git apply
mgifford’s picture

I do like the clearly red delete button associated with the screenshot in #255.

For colour blind screen reader users though it could be improved by adding a visual queue as well.

I haven't seen much progress on #606490: Drupal GPL icons - a softfacade initiative but it's a pretty important button...

ry5n’s picture

@kika Thanks. I will have time to work on this again early September.

eojthebrave’s picture

Status: Needs work » Needs review
FileSize
78.36 KB

I re-rolled the patch for changes to how the node form is built and worked on the tests that were failing. Those should all pass now but I have a feeling I broke more. The main test breaking change is that we removed the "Published" check box in favor of a select list with 0 / 1 instead of FALSE / TRUE. So tests that attempt to set $edit['status'] = FALSE | TRUE (which works create with a checkbox but not a select list).

I'm also a little bit concerned about the huge amount of CSS changes in this patch a lot of which seem like they may just be cleanup.

Anyway. Handing this to the testbot to see what it says.

Status: Needs review » Needs work

The last submitted patch, 1510532-260-create_content_form.patch, failed testing.

eojthebrave’s picture

Status: Needs work » Needs review
FileSize
77.76 KB

.

Status: Needs review » Needs work

The last submitted patch, 1510532-263-create_content_form.patch, failed testing.

Everett Zufelt’s picture

@ry5n

Is the source order node form, actions buttons, settings (fieldsets)? If so then the actions need to somehow come after the settings.

I think you're right, since legend is rendered by theme_fieldset it doesn't have the casing of theme_form_element to handle #title_display.

eojthebrave’s picture

Status: Needs work » Needs review

Well then. I guess it passes.

For reference in addition to re-rolling this for the new entity forms I also removed the "published" checkbox since there were two fields with the name 'status' and it was confusing things. So now we have just the select list next to the save button.

I also cleaned up some of the CSS for the edit summary, making labels bold and the last edited date italic and a few other minor CSS tweaks.

kika’s picture

Talked with @eojthebrave about likely split-off points for the path to be smaller and better digestable. First ideas:

- Push all code formatting / documenting changes to a separate patch
- Pull out global Seven CSS changes so they could be applied separately. That sub-patch could be a base for implementing primary and delete buttons everywhere. @bohjan: You mentioned there are issues about that?
- ...there are definitely more splitoff points...

kika’s picture

Quick code review

Small fixes

--- a/core/modules/file/lib/Drupal/file/Tests/FileFieldWidgetTest.php
+++ b/core/modules/file/lib/Drupal/file/Tests/FileFieldWidgetTest.php
@@ -300,7 +300,7 @@ class FileFieldWidgetTest extends FileFieldTestBase {
     // Unpublishes node.
     $this->drupalLogin($this->admin_user);
     $edit = array(
-      'status' => FALSE,
+      'status' => 0,
--- a/core/modules/forum/lib/Drupal/forum/Tests/ForumIndexTest.php
+++ b/core/modules/forum/lib/Drupal/forum/Tests/ForumIndexTest.php
@@ -67,7 +67,7 @@ class ForumIndexTest extends WebTestBase {
 
     // Unpublish the node.
     $edit = array(
-      'status' => FALSE,
+      'status' => 0,
-- a/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
@@ -170,7 +170,7 @@ class TranslationTest extends WebTestBase {
     // Unpublish the Spanish translation to check that the related language
     // switch link is not shown.
     $this->drupalLogin($this->admin_user);
-    $edit = array('status' => FALSE);
+    $edit = array('status' => 0);
     $this->drupalPost("node/$translation_es->nid/edit", $edit, t('Save'));
     $this->drupalLogin($this->translator);
     $this->assertLanguageSwitchLinks($node, $translation_es, FALSE);
@@ -181,7 +181,7 @@ class TranslationTest extends WebTestBase {
     $edit = array('language_interface[enabled][language-url]' => FALSE);
     $this->drupalPost('admin/config/regional/language/detection', $edit, t('Save settings'));
     $this->resetCaches();
-    $edit = array('status' => TRUE);
+    $edit = array('status' => 1);

Does these snippets have to be in?

Deeper questions

--- a/core/modules/node/lib/Drupal/node/NodeFormController.php
+++ b/core/modules/node/lib/Drupal/node/NodeFormController.php
@@ -106,19 +109,61 @@ class NodeFormController extends EntityFormController {
     );
 
     $form['additional_settings'] = array(
-      '#type' => 'vertical_tabs',
+      '#type' => 'fieldset',
       '#weight' => 99,
+      '#attributes' => array(
+        'class' => array('collapsibles'),
+     ),

Would it be cleaner to introduce a new container type here? (Ultimately we want to move away from the fieldsets #1168246: Freedom For Fieldsets! Long Live The DETAILS.). This could be a followup topic though.

Also, 'collapsibles' can easily be confused with FormAPI properties #collapsed and #collapsible.

--- a/core/modules/node/node.module
+++ b/core/modules/node/node.module
@@ -176,6 +176,11 @@ function node_theme() {
     'node_recent_content' => array(
       'variables' => array('node' => NULL),
     ),
+    'layout_form_3' => array(
+      'render element' => 'form',
+      'path' => 'core/modules/system/layouts',
+      'template' => 'layout-form-3',
+    ),

Is core/modules/system/layouts subdirectory really needed? All the core templates live in module directories.

layout-form-3.tpl.php: is -3 suffix needed? Could form-2col.tpl.php or something be more descriptive?

--- a/core/modules/node/lib/Drupal/node/NodeFormController.php
+++ b/core/modules/node/lib/Drupal/node/NodeFormController.php
@@ -54,6 +54,9 @@ class NodeFormController extends EntityFormController {
    * Overrides Drupal\entity\EntityFormController::form().
    */
   public function form(array $form, array &$form_state, EntityInterface $node) {

…

+    $element['delete']['#button_type'] = 'delete';
+    $element['preview']['#button_type'] = 'preview';

I has been discussed if it makes sense to move NodeFormController form overrides to a upper level, EntityFormController, at least #button_type properties. It could be part of a followup though.

--- a/core/themes/seven/reset.css
+++ b/core/themes/seven/reset.css
@@ -1,202 +1,391 @@
+/**
+ * @file
+ * Reset stylesheet for Seven admin theme

reset.css seems to be completely reformatted, it's very hard to see actual changes (what are likely not too numerous)

style.css and style-rtl.css likely warrant a separate review

kika’s picture

Visual review

There is not much to do for visual review, the progress so far is impressive. The spatial disconnect between top right status pane and low left "Save" widget is still a little bit puzzling but it should not hold this patch back.

New global bootstrap-esque form styling is nice. It might need a tiny bit of fine-tuning but it's definitely a go. Also liking how some of the initial controversial design choices are resolved. To make new styling to succeed it needs a separate issue (TBD).

Followup material: It would make sense to provide basic visual separation of primary, normal and delete #button_type in all themes including Stark.

Followup material: 2col responsible form layout is likely re-usable in other pages as well (other entity edit screens etc) and in non-form contexts as well so tech implementation should be as abstract as possible.

eojthebrave’s picture

I agree that we should split this up in to multiple patches, if nothing else it makes it easier to review.

The code changes in which 'status' is using 0 & 1 now instead of FALSE & TRUE are necessary because we use a select field now instead of a checkbox and the select field. We might be able to do something like $options = array(FALSE => 'un published', TRUE = 'pubslished') but I haven't actually tried that yet. Not sure if it would work since a select list isn't really a boolean value like a checkbox is.

I can't really speak to the name or location of the template file but. I'm assuming that it's in the system module instead of the node module so that it can be re-usable by other modules that want to implement similar patterns.

Regarding the #button_type stuff, I'm inclined to make that part of the follow up. Just to allow us to get this wrapped up and working before tackling ALL the entity forms.

Bojhan’s picture

@eojthebrave Could you split it up? I woulnd't know which parts, to split it up into.

kika’s picture

FileSize
60.14 KB
48.44 KB
59.4 KB
48.76 KB

Here's the version without totally rewritten themes/seven/reset.css. There are some small visual differences:

With rewritten reset.css

With old reset.css

kika’s picture

Here's the patch without reset.css changes. See spin-off issue #1747868: Modern CSS reset in Seven

Status: Needs review » Needs work

The last submitted patch, 1510532-274-create_content_form-_no_reset.patch, failed testing.

kika’s picture

Commit of #1137782: Remove unused CSS in Seven theme that uses desktop-sized layouts broke the patch. For now test it with

git reset 4787768 --hard
curl http://drupal.org/files/1510532-274-create_content_form-_no_reset.patch | git apply

This clearly shows how fragile are the monolithic patches like this.

eojthebrave’s picture

+++ b/core/modules/node/lib/Drupal/node/NodeFormController.phpundefined
@@ -106,19 +109,61 @@ class NodeFormController extends EntityFormController {
+    $pub_state = '';
+    $last_saved = '';
+    $author = '';
+    if (!$node->isNew()) {
+      $pub_state = (isset($node->status) && $node->status == 1) ? t('Published') : t('Not published');
+      $last_saved = '<span class="label">' . t('Last saved') . ':</span> ' . format_date($node->changed);
+      $author = '<span class="label">' . t('Author') . ':</span> ' . user_format_name(user_load($node->uid));
+    }
+    else {
+      $pub_state = t('Not published');
+      $last_saved = t('Not saved yet');
+      $author = '<span class="label">' . t('Author') . ':</span> ' . user_format_name($GLOBALS['user']);

Use $publication_state not $pub_state, we don't abbreviate variable names.

And this whole block could be updated to remove the } else { portion by just initializing the variables with the values from the }else{ statement.

+++ b/core/modules/node/lib/Drupal/node/NodeFormController.phpundefined
@@ -241,6 +275,37 @@ class NodeFormController extends EntityFormController {
+    // Add a nested fieldset to contain the save button and publication status

We're not really using a fieldset so this documentation should be updated.

+++ b/core/modules/node/node.jsundefined
@@ -33,14 +33,15 @@ Drupal.behaviors.nodeFieldsetSummaries = {
-      $context.find('input:checked').parent().each(function () {
-        vals.push(Drupal.checkPlain($.trim($(this).text())));
-      });
-
-      if (!$context.find('.form-item-status input').is(':checked')) {
-        vals.unshift(Drupal.t('Not published'));
+      if ($context.find('input').is(':checked')) {
+        $context.find('input:checked').parent().each(function () {
+          vals.push(Drupal.checkPlain($.trim($(this).text())));
+        });
+        return vals.join(', ');
+      }
+      else {
+        return Drupal.t('Not promoted');
       }

I'm not sure if this is needed or what the desired result is. For a few reasons. 1. The published field widget is now a select list not a checkbox. 2. We're not making use of the fieldset summaries in the new accordion (they're not accounted for in the mock ups)

+++ b/core/themes/seven/ie.cssundefined
@@ -1,12 +1,20 @@
+  margin-left: -1px;
+  margin-right: -1px;

Is there a reason to not do this using the shorthand. margin: -1px -1px 0;

+++ b/core/themes/seven/ie.cssundefined
@@ -1,12 +1,20 @@
+.accordion .collapsible legend {
+	border: 0;

Whitespace.

+++ b/core/themes/seven/style-rtl.cssundefined
@@ -1,6 +1,10 @@
- * Generic elements.

Lets save the css cleanup for another patch since this one is already going to be huge.

+++ b/core/themes/seven/style.cssundefined
@@ -57,8 +61,8 @@ dl {
 dl dl {
-  margin-left: 20px; /* LTR */
   margin-bottom: 10px;
+  margin-left: 20px; /* LTR */
 }
 blockquote {
   margin: 1em 40px;
@@ -82,17 +86,14 @@ small {

@@ -82,17 +86,14 @@ small {
   font-size: smaller;
 }
 sub {
-  vertical-align: sub;
   font-size: smaller;
   line-height: normal;
+  vertical-align: sub;

Again. This kind of cleanup should be for another issue.

+++ b/core/themes/seven/style.cssundefined
@@ -131,42 +132,36 @@ ul.menu li.expanded {
-quote,
 code {
   margin: .5em 0;
 }
-code,
-pre,
-kbd {
-  font-size: 1.231em;

I don't see how any of this applies to the changes for the content creation form.

+++ b/core/themes/seven/template.phpundefined
@@ -115,3 +115,44 @@ function seven_css_alter(&$css) {
+function seven_preprocess_layout_form_3(&$vars) {

Use $variables instead of $vars.

Uhm. There's a ton of CSS that I didn't review, mostly the form related stuff. For another day. :)

Powered by Dreditor.

kika’s picture

Re: tons of CSS. Let's split off unneccessary CSS changes ASAP.

ry5n’s picture

RE: #277: Some quick feedback on specific items from code review, FYI:

/core/modules/node/node.js
Fieldset summaries are still there, can be turned back on by adding a single class to the body (as some have expressed desire for the ability to do this). So JS has been updated.

/core/themes/seven/style.css
removing the `quote` selector because that is not an html element (let's just remove it now rather than clog the issue queue). The following lines with `code`, `pre` and `kbd` are gone because the new reset removed the need for them. I used normalize as per http://drupal.org/node/723392#comment-6260440 because I wanted to build the theme changes on top of something modern. I would consider that a blocker for this patch now rather than try to re-instate the old reset.

/core/themes/seven/ie.css
Margins are set individually on left and right because we don't want to accidentally reset top and bottom margins in case these values are not 0 in the main stylesheet.

RobW’s picture

Fieldset summaries are still there, can be turned back on by adding a single class to the body (as some have expressed desire for the ability to do this). So JS has been updated.

This is a non-standard way of configuring options in Drupal. To avoid adding extraneous javascript and extra html that's never seen, this should probably be a configuration option on the back end.

ry5n’s picture

You're right – that's a better approach. Unfortunately I'm not the best qualified to set that up. As we move this along I'm asking for help on all the back-end tasks. CSS is the only area I'd claim expertise.

Also RE: previous comments on the new template, naming is open for discussion – I always intended it to be renamed. Putting it in the system module was my best guess at making it re-usable by Scotch, after feedback from swentel, EclipseGc and JohnAlbin.

kika’s picture

Issue tags: +sprint

Adding sprint tags. We need help on this - anybody from dcmunich coding sprint?

mrfelton’s picture

This issue has grown to an incredibly long size, with a patch that is about as long. What specifically are you looking for help with at this point?

Bojhan’s picture

@mrfelton I think we need to figure out how to split it up into sensable parts.

mrfelton’s picture

I have cut out around 25% of this patch simply by removing los of the css cleanup and reorganisation that it was doing. Those are issues that should be addressed in a follow up patch that aims to standardise on css conventions and it not something that needs to be done as part of this issue. It just bloats the patch and makes it unreadable.

Attached patch applies cleanly against latest HEAD, and does exactly the same as the version in #274, but without the css style cleanup stuff (which was inconsistant anyway).

mrfelton’s picture

Status: Needs work » Needs review
FileSize
10.25 KB

There is no way that this giant patch will ever get committed in its current state - it tries to do far too much, and touches too many different areas of the system. So, I've started trying to break this patch down into separate parts, so that we can actually think about getting some of this stuff committed.

I propose that we keep this issue as something of a meta issue, and create separate issues for the individual components, which I see as the following:

  1. #1747868: Modern CSS reset in Seven
  2. #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently)
  3. #1751606: Move published status checkbox next to "Save"
  4. Move node forms vertical tabs to fieldsets.
  5. New form style for Seven (all the lovely chrome/blueprint type stuff)
  6. Implement Core's new Layout system (not sure how far along core is with this yet) to provide 2 column layout for node forms.

That may or may not be a complete list of the different aspects of this patch (let me know if I have missed anything).

First thing that I have is a new patch that handles just the new #button stuff. It does the following, which may or may not be too much for a single patch. I'd like to get some thoughts on that before creating a new core issue for it.

1) Adds additional logic into theme_button that injects new button and button-* specific classes on buttons that have the #button_type attribute set.

2) Sets #button_type of 'delete' and 'submit' on all EntityForms in EntityFormController.

3) Fixes the user account edit form so that it uses a standard 'delete' button instead of one called 'cancel' (this was marked as a @todo in ProfileFormController, and is needed to ensure that the delete button also get the button-delete class.

4) Adds basic button element styling for seven. Currently, it styles the buttons in the blueprint-esque styles as per the other patches in this thread. In some ways it may make sense to remove a lot of that css from this patch and do that in a wider Seven style update patch (point #5 above), although that would mean we would need some temporary styles for the new buttons and given that the goal is to make them look like they do in this thread I'm not sure there is merit to that approach or not.

The result of all that is the ability to use #button_type to give form buttons special classes, some nice new classes on form entity form submit buttons, and some styling in the seven theme to make use of the new classes so that form buttons are styled nicely.

---

Unfortunately, by breaking it down into smaller patches like this, I suspect that we may end up with several patches that are dependant on one another and need to be applied in a certain order. But, I still think we stand a much better chance of making progress this way.

kika’s picture

@mrfelton I think the split approach is solid. Please post #286 to it's own issue or even better -- post it as a followup on #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently). For now, post it with all the fancy styling but it would be nice to remove that fluff and just take original Seven buttons and color them bit more blue (that CSS might even exist in current Seven style.css) and red. Yes, those are temporary styles but it's ok.

Form style touchup (incl. buttons) should go to the separate issue.

mrfelton’s picture

ry5n’s picture

@kika, @mrfelton Thanks for working on this! (I’m sorry it’s so monolithic at the moment.) #286 covers most of the parts.

FYI there are still layout changes to complete (see #254) which means possibly a different approach to the layout template.

@Everett Zufelt Source order is 1) node form fields, 2) settings sidebar 3) actions. Tab order seems to be correct in my testing.

mrfelton’s picture

Status: Needs review » Needs work

New issue created for the publishing options #1751606: Move published status checkbox next to "Save". List of tasks now stands as:

  1. #1747868: Modern CSS reset in Seven
  2. #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently)
  3. #1751606: Move published status checkbox next to "Save"
  4. Move node forms vertical tabs to fieldsets.
  5. New form style for Seven (all the lovely chrome/blueprint type stuff)
  6. Implement Core's new Layout system (not sure how far along core is with this yet) to provide 2 column layout for node forms.
xmacinfo’s picture

Jumping in from another issue. Work being done here is impressive! Thank you very much.

Looks like the Save and Delete will have subdued colors, not as bright as the latest screenshots. I am eager to see what it will look like.

As for the Delete button format, do we need a full size button (last few screenshots) or a link (summary screenshot)? We might need to revise the summary screen if we need to use full size buttons.

mrfelton’s picture

Status: Needs work » Needs review

New issue created for the updated form style #1751754: Implement new form style for Seven, based on blueprint mockups.. List of tasks now stands as:

  1. #1747868: Modern CSS reset in Seven
  2. #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently)
  3. #1751606: Move published status checkbox next to "Save"
  4. #1751754: Implement new form style for Seven, based on blueprint mockups.
  5. Move node forms vertical tabs to fieldsets.
  6. Implement Core's new Layout system (not sure how far along core is with this yet) to provide 2 column layout for node forms.

Sorry, but all these patches are interdependent - I really can see no way around that.

kika’s picture

> Implement Core's new Layout system (not sure how far along core is with this yet) to provide 2 column layout for node forms.

You can check out current Blocks & Layouts UI: http://drupal.org/sandbox/eclipsegc/1441840 "Admin > Structure > Blocks > Block Library" to see how eclipsegc acheives the 2col layout, I have not yet looked into it.

EclipseGc’s picture

oh, still totally cleaning that up, not really trying to replicate this so much as be close to it in spirit. Happy to swap my own layout long term when this goes in if it's reusable.

Eclipse

jide’s picture

Implement Core's new Layout system (not sure how far along core is with this yet) to provide 2 column layout for node forms.

THAT would be awesome !

Everett Zufelt’s picture

Please make sure that any relevant follow up issues receive the 'needs accessibility review' tag.

kika’s picture

@mrfelton Any chance to see "5. Move node forms vertical tabs to fieldsets."? :)

mrfelton’s picture

@kika, Is it worth doing that until we know more about how the layouts will actually be implemented? Also, is there still open discussion about wether fieldsets are actually the right thing to use for this? Is there not a better scemantic HTML5 element that we could exploit that might reduce the need for so many browser hacks working around the cross-browser oddities of fieldsets?

kika’s picture

mikkopaltamaa’s picture

I think that the design proposed on issue summary is great.

However, I would change one small thing, which (IMHO) makes it a even more intuitive and easier to use. I would ghange the layout so that the meta information would be on the left side and the node fields on the right side:

Switched layout

Wim Leers’s picture

#300: what's your reasoning?

pjcdawkins’s picture

I guess #300 (proposing metadata left, content right) should be answered.

The fields you always need to edit for a new node are the content (e.g. title and body), whereas metadata can be ignored if there are sensible defaults. So you'd want to see, and read, and tab into, the content elements first.

Tabbing proceeds from left to right (in both LTR and RTL HTML, as far as I know). Additionally, if you're editing in a narrow version of your responsive layouts, the right-hand column drops below the left. Or, if for some reason the responsiveness of the design isn't working, and you have to scroll horizontally, it's the left-hand part of the page that is shown first.

In summary: users would want to edit content before metadata, and (although this is perhaps a flaw) left almost always comes before right.

xmacinfo’s picture

#300: This left-layout is nice to the eye, but no, the meta column on the left adds a lot of confusion. The arrows pointing to the content area next to Taxonomy and other tabs give the false impression that clicking the arrow will change the content area.

We don’t have these problems with the original right-layout.

That being said, I am eager to play with the new content creation form in the future.

xmacinfo’s picture

#302: In left to right layout (ltr), for many languages, the meta column would be expected on the left. But that may be dealt with in another issue.

jide’s picture

I don't agree, and I think it's the opinion of most people here: In LTR, we have the most important things first, the content, and then the secondary things, the meta. This is not an application / navigation sidebar.

webchick’s picture

Let's not make additional design tweaks until the actual spec here is implemented.

Anyone up for giving that a stab?

mikkopaltamaa’s picture

#301: Some arguments supporting metadata before node fields:

1) Metadata is often presented before the actual content (like the issue information at the top of this page, or the commenting form below).

2) Not so easy to just click publish and forget filling the metadata (that happens quite often for me).

3) Looks better and simpler (personal opinion).

mikkopaltamaa’s picture

#303: This one is easy to fix. You could change the icons to be on the left side of the accordion labels. Example:

Note that browsers render the HTML5 summary element so that the icon is before label, and the same for the old fieldset element. That is a one good reason for swiching the icons to the left side, and it also solves the mentioned pointing problem.

Or you could use plus icons or something else. Examples:

You could also change the accordion into vertical tabs and add a new tab for node fields. That way the metadata fields would have more space and might be easier to fill for a user.

These are just suggestions, I don't have any strong opinions on this.

Bojhan’s picture

Could we focus on solving the issues outlined in http://drupal.org/node/1510532#comment-6389382. I appreciate the comments considering changing the design.

However I want to be realistic here, although these comments are oke - we are not making any movement on the actual code, lets focus on getting the basis in. Because if we don't get any movement on this soon, we will not get it in in time for code freeze which is only like 10 weeks away.

jide’s picture

I may give this a stab in the next few days.

kika’s picture

For #290, task 6 ("Implement Core's new Layout system to provide 2 column layout for node forms."), see #1787634: [META] Decouple layouts from themes

effulgentsia’s picture

Re #311: I don't think it's close enough. I hope we're able to get a Layout system committed to core before feature freeze (Dec. 1), but a) there's no guarantee of that, and b) it won't be much before then. It makes a lot more sense to me to try to get this issue finished independently of the Layout system and before the feature freeze, then once we have a Layout system, open a follow up issue to clean up the code to use it, which we'd have until code freeze (Apr. 1) to do.

effulgentsia’s picture

Issue summary: View changes

further cleanup

iantresman’s picture

The two most important things for me, are that

  1. the Create Content page can optionally have labels in a column
  2. the layout of the Create Content page, may be different to the node view, with fields in a different order.

For example, I often want to Create Content thus:

Date: [____-_______-_______]
Name: [_______________________________________]
Address: [_______________________________________]
[_______________________________________]
Postcode: [_____________]

And then I want to display the node thus:

Contact:
     John Doe
     5 High Street
     London W1A 1AA
     (31 Oct 2012)

iantresman’s picture

Issue summary: View changes

Update with latest implementation plan and todos from http://drupal.org/node/1510532#comment-6391940. Remove some old todos, keeping as much relevant info as possible. Emphasize approaching feature freeze.

yoroy’s picture

I added the list of sub-issues from http://drupal.org/node/1510532#comment-6391940 to the issue summary. It's these:

  1. #1747868: Modern CSS reset in Seven
  2. #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently)
  3. #1751606: Move published status checkbox out of vertical tabs, into 'form actions group' at bottom of node forms, and make it a select
  4. #1751754: Implement new form style for Seven, based on blueprint mockups.
  5. Move node forms vertical tabs to fieldsets.
  6. Implement Core's new Layout system (not sure how far along core is with this yet) to provide 2 column layout for node forms.

Aspilicious tells me the biggest potential blocking issue is maintaining accessible source-ordering with a 2-column layout when viewed in the overlay.

Would be great to see if we can untangle this in a way that lets us put the bare minimum needed in place before feature freeze…

yoroy’s picture

Issue summary: View changes

update with sub-issues

jstoller’s picture

Sorry I'm late to the party, but I want to make sure this issue takes into account the outcome of #218755: Support revisions in different states. Namely the new concept of a default revision, which is no longer necessarily the latest revision. The current mock-ups don't indicate how that will be handled. #1776796: Provide a better UX for creating, editing & managing draft revisions. was created to deal with this and appears to have a good deal of overlap with what's being discussed here, though it goes a bit beyond just the content editing page. These issues seem very interconnected.

On a related note, the "Create new revision" check box is extremely problematic. You can see #563550: Change "create new revision" check box for a discussion of this dating back to 2009. It just doesn't play well with workflows. At a minimum the phrasing should be flipped and replaced with a "Overwrite active revision" checkbox (or something along those lines). And this would need to be physically close to the submit button. Realistically, we'd all be better off if it just went away all together.

Even given our current two-state workflow, there are a number of options I see we need to consider.

  1. Save new revision as draft
  2. Save new revision as published and make default
  3. Save new revision as draft and make default (this unpublishes the content)
  4. Overwrite active draft revision as draft
  5. Overwrite active draft revision as published and make default
  6. Overwrite active draft revision as draft and make default (this unpublishes the content)
  7. Overwrite active published revision as draft (this unpublishes the content)
  8. Overwrite active published revision as published

If #1777388: Support arbitrary workflow states is implemented (please, please, please) and we have an arbitrary number of workflow states to deal with, then this can get real complicated. In #1776796: Provide a better UX for creating, editing & managing draft revisions. I proposed moving the publishing and unpublishing of existing revisions from the edit screen to the view screen. That would allow us to ignore 3 & 6. If we can accept that saving revisionable content will always create a new revision, then we can ignore 4-8. The two remaining options are easily handled with the drop-down selection button shown here, which will also easily scale to more advanced multi-state workflows.

lpalgarvio’s picture

lpalgarvio’s picture

Issue summary: View changes

Linking Seven reset issue to original issue the one cited here was a duplicate); also adding a link to an issue webchick pointed to re: implementing layout.

ry5n’s picture

Issue summary: View changes

Add link to new sub-issue: Move node form’s vertical tabs to collapsibles (http://drupal.org/node/1838114)

ParisLiakos’s picture

Status: Needs review » Needs work

dont see a patch to review?

cosmicdreams’s picture

would this issue be able to be implemented after feature freeze? I'm guessing no

ry5n’s picture

@rootatwv @cosmicdreams This became a sort of task meta-issue because the original patch was big and hard to review. We have a number of sub-issues in various stages of completion. Would love help and/or reviews!

ry5n’s picture

Issue summary: View changes

Created issue for implementing layout.

ParisLiakos’s picture

Title: Implement the new create content page design » [META] Implement the new create content page design
Status: Needs work » Active

Thanks. thus changing title

ParisLiakos’s picture

Issue summary: View changes

Cleaned up list of sub-issues, marked feature issues bold and added issue type to focus work before feature freeze.

ry5n’s picture

Issue summary: View changes

Update issue status for major sub-issues

ry5n’s picture

I need help in particular with [feature]#1838114: Change node form’s vertical tabs into details elements. If this design is going to make it into D8, I need help from someone who knows Drupal's test infrastructure to help me fix failing tests in the latest patch.

I also need reviewers for #1751606: Move published status checkbox next to "Save" and #1838156: Implement the two-column layout for the new create content page which have green patches but need to get to RTBC before the end of the month.

ry5n’s picture

Issue summary: View changes

Updating sub-issue statuses

Gaelan’s picture

XmhO’s picture

Hi,

I don't know If this feature has already been suggested or not, but we definitely need a fixed "Save" button.
I can see that sometimes in your workflows, there is a "Save" button in the sidebar, and sometimes not. Maybe I misunderstood something?
But let's admit we have a really huge content type -for instance- and that in a node/%/edit page, you only want to edit the title. Isn't it horrible to scroll the whole page to save the content?

andypost’s picture

Suppose this issue #1875994: [meta] EntityDisplays should be attached to summary with postponed #1875992: Add EntityFormDisplay objects for entity forms

andypost’s picture

Issue summary: View changes

Updating major issues statuses

swentel’s picture

Issue summary: View changes

Updated issue summary.

jstoller’s picture

I thought people here should be aware of #1838918: Add 'published' timestamp to nodes, which is just waiting for a review from the usability team. Once committed, the new field added by that patch will need to be folded into the work being done here.

jstoller’s picture

Issue summary: View changes

Updated issue summary.

swentel’s picture

Issue summary: View changes

Updated issue summary.

omaster’s picture

Started looking at this sort of issue but we have been looking at it from an overall viewport for all pages. So we have started to develop patches to make the important buttons always visible on a page. early rough patches are available here. Obviously needs more work and input.
http://drupal.org/node/752482#comment-7159496

eigentor’s picture

I have posted some followups, which concern with balance between "meta" fieldsets and the content edit part and the bahavior of the meta fieldsets.

#2009932: Implement accordion behavior for meta fieldsets on node form
#2009930: Do not keep meta fields of node form open when they have a value

I am hesitant to add them to the issue summary, want to see how the reaction is first.

dlu’s picture

Component: base system » node.module

Moved to node.module per #2050763-16: Refine "base system" component (notes on refactoring of "base system" category here: https://docs.google.com/a/acquia.com/spreadsheet/ccc?key=0AusehVccVSq2dF...).

xmacinfo’s picture

Version: 8.x-dev » 9.x-dev
Priority: Major » Normal

Time flies fast and it’s time to set this to Drupal 9!

webchick’s picture

Version: 9.x-dev » 8.x-dev
Priority: Normal » Major
Status: Active » Fixed

Wait, huh? Isn't it instead time to call this fixed? This was done a long time ago.

klonos’s picture

Well, according to #319 (and the issue summary) we still have two feature requests and two tasks left before we can call this fixed. So I guess either set this back to active and wait for these last remaining issues to be fixed before you set this one to fixed as well, or move all issues to D9 and call it a day.

jthorson’s picture

klonos: All of the referenced issues (except 1751754) have initial commits, and are only open for minor followups ... I think the meta issue has outlived its purpose. :)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

xjm’s picture

Component: node.module » node system
Issue summary: View changes
yched’s picture

Gábor Hojtsy’s picture

Version: 8.0.x-dev » 8.0.0
Issue tags: -sprint

Thanks, removing from UX sprint now.