Vertical tabs shifted in node type form

Suggested layout improvement in bartik css files.

This appears to work but I need to know why the statements above were added in content.css and blocks.css

Added: a minor style repair suggestion in "vertical-tabs.component.css"
.vertical-tabs__pane {
margin: 0;
border: 0;
}

The block ol, block ul, and region-content ol, region content ul are used in many places, it is not clear how it affects all of them.

(This is my first ever uploaded patch, so bear with me. I forgot to add the change in vertical-tabs.component.css to the patch and recreated the patch. I have to find out how to delete the wrong patches)

@Cheviot that is a good start to narrow the problem and looks like you got the tabs under control as 2 spaces, so thanks for that:) If we can get a more specific selector to help the vertical selector avoid .block and .region-content ol/uls that would be ideal then we don't have to adjust their typographic behaviour.

Any chance you can give that a go?

Why are we doing this ? Seven is the admin theme, Bartik is not an admin theme and we should not be tinkering with it IMHO to try and make it one.

@bill richardson : Bartik needs to support content creation. If a content entry user does not have permission to see the admin theme they will only see the front end theme for the node edit/add pages. So we should most definitely fix this.

Thanks for the patch. Found some regression issues after #4 patch. Please check screenshots.

Before

After

Before

After

Before

After

@joelpittet
Sure I can help, that's why I attended Sprint Sunderland to get involved and to get a good idea about version 8. I still have to learn a lot though..
I'm on IRC also under the same user name on #drupal-contribute.

@Manjit.Singh I have a look at this, I expected this to happen somewhere, but first the 'top' level has to be right and then the ul/ol parts after this can be addressed more consistently. Thanks for the sample.

At first look it appears that we need here some extra selectors. Currently they come from system.theme.css. These are not vertical tab issues (block), these are item lists issues. I have a possible patch for it.

@joelpittet, does an issue exists related to "item lists" to cover the samples mentioned by @Manjit.Singh? It is imo not really a vertical tab issue.

At first look it appears that we need here some extra selectors.

@Cheviot Yeah, I was also thinking the same, If we can add extra selectors in vertical tabs for solution. and Also that will not cause any regressions.

But first @joelpittet please verify the issues that i have reported in #8.

@bill richardson: the components used on admin sections are not specifically meant for admin sections. We cannot know all the use cases modules are creating which means they might be used even for anonymous users in some use cases!

@joelpittit please find a proposed patch to compare against the note of @Manjit.Singh.
This one corrects the 'list items' and 'bread crumbs' by adding a selector in the appropriate css files.

There is also a small issue with the image in an article content type. The list creeps into the image right border. I'm not sure if issue should be raised against the image or the list-items themselves.

It may be possible to fix this with a new selector in items-list.css like this, which would let the list block flow properly next to the image.
.field-item ul {
display: inline-block;
}

I have not found any issue with an additional inline-block statement. A patch is ready for this but I give some time for other to comment.

triggering test bot :)

Unassigning so it is clear for others to step in and review @Cheviot's work.

+++ b/core/themes/bartik/css/components/breadcrumb.css
@@ -7,3 +7,6 @@
\ No newline at end of file

At least new line needed to the end of this file :)

regarding list-items, patch seems fine !!

@Cheviot Can you please merge the #14 and #4 patches so that we can test vertical tabs along with list items and other regression issues.

@Manjit.Singh, a patch merging #4 and #14

contains changes to:
blocks.css
breadcrumbs.css + the newline mentioned by @lauriii
content.css
items-list.css
vertical-tabs.component.css

Here's a review of every line. I've not manually tested because much hasn't changed since I reviewed in #5

1. +++ b/core/themes/bartik/css/components/content.css
   @@ -229,8 +229,8 @@ ul.links {
   -  margin: 1em 0;
   -  padding: 0 0 0.25em 15px; /* LTR */
   +/*  margin: 1em 0;
   +  padding: 0 0 0.25em 15px; /*LTR */
   

   Can you leave this uncommented and still resolve the problem?

2. +++ b/core/themes/bartik/css/components/item-list.css
   @@ -2,7 +2,9 @@
   +.item-list ul {
   +  margin: 0 0 0 0.9em;
   +}
   

   This is quite a generic selector that can effect many things. Is this change needed?

3. +++ b/core/themes/bartik/css/components/item-list.css
   @@ -10,3 +12,6 @@
   +.field-item ul{
   +  display: inline-block;
   +}
   

   Be sure to copy the coding standards from the other code in the document, that means putting a space before the curly brace and the last selector.

   Also, this is a very generic selector considering the problem is with vertical tabs the, IMO you should only be adding/changing selectors that target that element.

4. +++ b/core/themes/bartik/css/components/vertical-tabs.component.css
   @@ -14,3 +14,7 @@
   +.vertical-tabs__pane{
   +  margin: 0;
   +  border: 0;
   +}
   

   Is this actually doing anything? Also coding standards.

triggering testbot for #21

@joelpittet, no significant changes, it is a merge on request of @Manjit.Singh and I added the missing last new-line as mentioned by @laurii

>>Can you leave this uncommented and still resolve the problem?
Yes, this should actually indeed be left out
+.item-list ul {
+ margin: 0 0 0 0.9em;

The only important one needed seems "+ display: inline-block;" to let it flow around the image better as seen in an article content (content.css -> .field-type-image img, .field-name-field-user-picture img).

>>Also, this is a very generic selector considering the problem is with vertical tabs the, IMO you should only be adding/changing selectors that target that element.
I take your point, not sure what's the best way, I tried various but this is the only one I could find working consistently across the many admin menu's. Do you know of a case where my current suggestion with vertical-tabs goes wrong on another level? That would be helpful for me.

Thanks for pointing out the coding standards, I'm a newbie and did not notice it. You want me to send a new patch to clean it up?

Sorry didn't mean to be short, was disappointed with my own comment not being as clear as it could've been the first comment.

Totally send up a new patch.

@joelpittet, no problem. here is a new patch with my suggestion. I removed some and checked if it affects anything, going through all menus I found no issue. Still no idea how it may affect vertical-tabs somewhere else. How are the changes actually tested for all possible situations?

@Cheviot try not to scope creep CSS changes beyond the issue summary. It's hard not to do (I know I get caught doing that all the time). But someone else in a different issue may be trying to resolve that other problem. (in this case being images and breadcrumbs in the content)

1. +++ b/core/themes/bartik/css/components/breadcrumb.css
   @@ -2,8 +2,10 @@
   -
   ...
   +.breadcrumb {
   +  padding: 0 0 0.5em 1.3em;
   +}
   

   These changes don't seem relevant to the patch? Why are we styling breadcrumbs here?

2. +++ b/core/themes/bartik/css/components/content.css
   @@ -227,11 +227,6 @@ ul.links {
   -.region-content ul,
   -.region-content ol {
   -  margin: 1em 0;
   -  padding: 0 0 0.25em 15px; /* LTR */
   -}
   

   I think this one is actually meant to target the body content UL/OL. But maybe there is a more specific selector that can be used for that?

3. +++ b/core/themes/bartik/css/components/item-list.css
   @@ -2,11 +2,12 @@
     * Visual styles for Bartik's item list.
     */
   -
    .item-list ul li {
   -  margin: 0;
      padding: 0.2em 0.5em 0 0; /* LTR */
    }
   

   Shouldn't need to remove this whitespace and can't see how the margin: 0 reset should have any effect on the vertical tabs.

4. +++ b/core/themes/bartik/css/components/item-list.css
   @@ -2,11 +2,12 @@
   +.field-item ul {
   +  display: inline-block;
   +}
   

   This selector is very generic, why is it needed?

5. +++ b/core/themes/bartik/css/components/vertical-tabs.component.css
   @@ -14,3 +14,7 @@
   +.vertical-tabs__pane {
   +  margin: 0;
   +  border: 0;
   +}
   

   This is the only change I see in the patch that is likely absolutely necessary and the region-content one likely need just more specificity.

trigger testbot :)

NW as per #28

#joelpittet Ah, indeed, good point. I was a bit unsure about this and wondered where one thing ends and the next starts.

From one 'repair' (vertical-tabs at 15em changed to 0 via vertical-tabs__pane) a next one appeared as seen in #8.

Lets first find a definite solution for vertical-tabs and agree on this before we go further with anything else. New issues can then be raised as a new issue, is that ok with you? I reset Bartik on my side to the current default install plus the current vertical tab change and we go from there.

As Bartik already has the vertical-tabs.component.css, let's add inheritance for vertical tabs in node-form.

I am postponing this until further steps have been taken on #2548805: Add sensible base layout styles for lists in Bartik. which is currently RTBC. This issue affects that one and also solves this one in return. Thanks.

This looks to me to be fixed. Please re-open if I'm wrong.