Make displays appear like local tasks in the Views UI instead of custom UI design

Oh yes please. That looks so much better.

I love it as well. Its just way more clearer!!

Its far less crowded! I'm curious what you think about removing the advanced toggle?

I agree, the suggested design is much clearer and brings the ui in line with the rest of the project.

@dawehner I suggest leaving the 'Advanced' toggle as it currently is (for non-power users, as it makes this screen less overwhelming):

• Toggle'able
• Defaulted to collapsed

@mikeburrelljr
Well fair, even I don't believe its for power users. Stuff like enable ajax or contextual filters are a common feature.

About the proposal, it seems to be that placing the view into a secondary level local task might fit more into the way how people think about it.
Does someone has an opinion about it?

Interesting, that might actually be better. However would we still have main tabs then? Its very unordinary to only do secondary tabs.

@roy do you have a source file for this?

@yoroy I'd love to jump on a hangout with you (and record it to share with everyone here) about some of the edge-cases of the current UI functionality that I'm concerned about.

I'm really optimistic about your proposed changes, there are just a couple more details to iron out.
I'll still be recovering from Drupalcon all week, but let me know when would work for you.

@dawehner you mean listing the displays in secondary tabs? I can see how that maps better to user expectations.

Yes exactly.

Some of the points @tim.plunkett mentioned are stuff like the fact that we mark the displays itself as "changed", so you know on which ones you worked on.

Should we have the mobile layout side by side with this issue since I would like to see accordion arrow after titles like Fields in mobile version and not just after the Advanced category?

The ui update seems fine but I think moving the displays to the tabs is bit too much. How are we going to add styling to tabs when view will have unsaved changes like deleted displays or displays with changes? IMO it lacks some contrasts, seems little dull.

Per discussion with yoroy and Bojhan, I updated this to conform to the secondary tabs style. The only outlier is now the validation (warning for unsaved changes) as shown in the mock.

Given that the secondary tabs include a horizontal rule which serves to create a seperation we no longer need the gray background in the display name container.

Good point, the one shown in my mock is both active and unsaved. Unsaved could just be the text color or perhaps we could also add the warning icon as a :before pseudo element.

#2566827: move views_ui.admin.theme.css to seven would help us work on this in 8.1+ as Seven markup and CSS will not be frozen in minor versions.

Worked on it a bit. Its quite a mess since its all over the place in Views UI. I need some direction from Lewis how we can integrate our regular tabs styling, instead of using Views.

Daniel & Tim mentioned to change the html structure to use the secondary tabs we haven in normal admin pages.

This entails the following:

• Look at the HTML structure used for secondary tabs on normal admin pages
• Replicate this HTML structure in Views

Might be an easy task for a novice to pickup, a few template changes.

When someone want to work on that I'm happy to asist or maybe I'll try things out when I'm bored. Its marked as sticky issue now :)

Just a start. For example though the active flag doesn't work yet.

Screenshot of the current patch. Definitely needs some UI work :) Do we still want both the primary tabs look (implemented here) and the secondary tabs look (sort of starting to be implemented here)?

Also updated issue summary with template and before/after. Although not sure the after is what we are still looking for (both primary and secondary tabs?).

Just talked to @mirom if he is still working on it. He said he will post a patch today :)

In this patch I combined all previous patches and I fixed theming from #31, but it still needs some work to do.

Changes Miroms flag to needs review so someone reviews what he did (then we can put back to needs work)

This needs a reroll.

Im at the Baltimore Con and im gonna try to play with this. (as confident as I can make that sound...

Ahahahah, i just want to wish you good luck. Please make it move forward, i fully support you! <3

I have 'rerolled' this patch to work on 8.4 I am going ahead and posting it on this thread and going to lunch but i hope to revisit it to make progress later today. ( I had to do a lot of manual refactoring because the arrays had all been changed over to brackets)

So I think I need to turn this back over to someone more familiar than me.

I am confused by what was intended to happen with the button for adding a display. The Ajax creating the Add button and drop down is broken, but it looks like it is broken because it was being recreated as secondary tabs. I am not really sure if I should be trying to restore the button or make the tabs work (and I'm not sure what the secondary tabs should be.

Attached is a screenshot of what its doing now (with the post 50 version of the patch)...

@epophoto, thanks for moving this forward. A few points:

1. When you add a patch, you usually set the status to NR so that the testbot will be triggered. From your comments, I do not see any reason not to do this, so I will change the status for you.
2. I guess the previous patch (like many others) needed a reroll because of #2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase. The issue description there links to a script that can help convert from the long array syntax to the short one.
3. You can include images inline in issue comments. Here is your screenshot:

Rerolling...

Manually fixed conflicts on:

• core/themes/stable/css/views_ui/views_ui.admin.theme.css
• core/themes/seven/css/components/views-ui.css
• core/modules/views_ui/js/views-admin.js
• core/modules/views_ui/css/views_ui.admin.theme.css

Made also the changes to core/modules/views_ui/js/views-admin.es6.js and generated views-admin.js using that.

1. +++ b/core/modules/views_ui/css/views_ui.admin.theme.css
   @@ -289,18 +289,13 @@ td.group-title {
   +
   

   unnecessary extra line

2. +++ b/core/modules/views_ui/css/views_ui.admin.theme.css
   @@ -384,26 +364,21 @@ td.group-title {
   +  /* border-bottom: 3px solid #004f80; */
   

   If we dont need this line, remove it.

3. +++ b/core/modules/views_ui/css/views_ui.admin.theme.css
   @@ -428,10 +403,12 @@ td.group-title {
   -  margin: 12px 12px 0 12px
   +    padding-top: 15px;
   

   indentation is off

4. +++ b/core/themes/seven/js/nav-tabs.js
   @@ -17,7 +17,10 @@
   +      console.log($tab);
          var $tabs = $tab.find('.tabs');
   +      console.log($tabs.outerHeight());
   +      console.log($tabs.find('.tabs__tab').outerHeight());
   

   These need to go =)

5. +++ b/core/themes/stable/css/views_ui/views_ui.admin.theme.css
   @@ -428,10 +402,12 @@ td.group-title {
   -  margin: 12px 12px 0 12px
   +    padding-top: 15px;
   

   indentation is off

6. +++ b/core/themes/stable/css/views_ui/views_ui.admin.theme.css
   @@ -692,28 +658,15 @@ td.group-title {
    .view-preview-form__title {
   -  background-color: #e1e2dc;
   -  border-bottom: 1px solid #ccc;
   -  margin-top: 0;
   -  padding: 8px 12px;
   +    display: inline;
    }
   

   indentation is off

7. +++ b/core/themes/stable/css/views_ui/views_ui.admin.theme.css
   @@ -797,14 +750,16 @@ td.group-title {
   +.views-display-top {
   +    padding-top: 5px;
   +}
   

   indentation is off

OK a bit of progress on this.

• The changes to the js file was breaking things pretty badly, so I have removed those changes for now.
• Cleaned up what i mentioned on #57
• Added the border bottom so it looks the same as on other tabs.
• Re: #15: About changing the color of the modified display tab, currently we do not have any classes to target with CSS to do this.

Here are some screenshots as to where we stand with the current patch:
Before

After

Super cool! We can probably use a better dropdown style if we use the standard button one?

Yes @Bojhan, we definitely should do something about that!

I'm thinking we could perhaps reuse the dropbutton widget we use on other places, for example on the Block layout page... only problem there would be that the + sign would not be there so I'm not sure about this.

Do we have any other places in core where we have a dropdown to add new things?

In the meantime, some more cleaning up of trailing whitespace, console.log calls etc.

Also, I'm pretty sure we should not be making changes to stable theme's css...

Some more patch cleaning up... this time

core/modules/views_ui/src/ViewEditForm.php:

• Restored the actions() method, which removes the delete link and adds the cancel button, as its not in scope here.
• Restored a bunch of CS fixes, not in scope and we keep the patch smaller and easier to review.

Removed commented line on core/themes/stable/css/views_ui/views_ui.admin.theme.css

Some more progress on this:

1. Added a new css class so we can target tabs where there are changes, could only add it to the <a> tag, so I had to rework a bit the styles to add the border bottom to the links themselves.
2. Had a go at the + Add new displays link and dropdown, mainly positioning it properly and keeping the hover state while the dropdown is active.
3. Added the change in border bottom on hover like on the rest of the secondary tabs in Drupal

When it comes to the dropdown, I believe we should either get a design for it that matches core more closely (though not sure we have an example of this anywhere), or leave it as is?

Still to do in any case (at least), refactor changes so we do not touch stable theme.

Fix for the failing tests.

Just looking at the interdiff:

-      if ($view->changed_display[$id]) {
+      if (isset($view->changed_display[$id]) && $view->changed_display[$id]) {

Wouldn't it be simpler to use if (!empty($view->changed_display[$id]))?

Thanks @benjifisher - good call (fixing it in this patch).

I've spent some time cleaning up where we were making the changes.

So in this patch:

• Moved all the changes being done to stable into seven.
• Moved all the changes being done in core/themes/seven/css/components/tabs.css into core/themes/seven/css/components/views-ui.css
• Removed unnecessary change to core/modules/views_ui/css/views_ui.admin.css
• Removed unrelated change out of core/modules/views_ui/src/ViewPreviewForm.php
• Removed unrelated CS fix on core/themes/seven/js/nav-tabs.js

I have had to rework a bit the CSS of course due to the fact that this patch is now making Seven theme override Stable so we can get this into a point release.

I am not sure whether we should also move changes we're making to core/modules/views_ui/css/views_ui.admin.theme.css into Seven theme.

I want to point out that this patch currently does a lot more than just tackle the way we present the view display tabs, so perhaps we should update the issue title / description.

$ git diff --stat 8.5.x
 core/modules/views_ui/css/views_ui.admin.theme.css | 101 +++++++++++++++++++++++++----------------------------------------------------------------------------
 core/modules/views_ui/src/ViewEditForm.php         |   1 -
 core/modules/views_ui/src/ViewFormBase.php         |   3 +++
 core/themes/seven/css/components/views-ui.css      | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
 4 files changed, 140 insertions(+), 106 deletions(-)

So now we're just touching 4 files instead of 10. Hopefully this will make the patch easier to review & improve upon =)

Here's how the view edit page is looking with this patch:

Had a chat with @Bojhan to figure out what to do about the add button, this is what we should do:

https://groups.drupal.org/files/8.dropdown-and-popover.png

For reference, please see: https://groups.drupal.org/node/283223#Dropdowns_and_Popovers

I'm having a look at this and will review.

+++ b/core/themes/seven/css/components/views-ui.css
@@ -277,6 +335,26 @@ details.fieldset-no-legend {
+.view-preview-form__title {
...
+.view-preview-form .form-item-live-preview {
+  position: static;
+  right: auto;
+  top: auto;
+  margin-top: 0;
+  margin-left: 1em; /* LTR */
+}

The tick box does not align quite right, it is slightly lower than the other things on the line.

I found that there was a square white rectangle around the dropbuttons. The resolution was to override the background colour and set it to transparent.

The tick box does not align quite right, it is slightly lower than the other things on the line.

@dimensional_dan, did you work in this comment too? I do not see any of that in your interdiff. Can you provide a screenshot of how it looks with patch? Then I get in impression of the result without applying the patch.

@Sutharsan: I did not make any changes in regards to the alignment issue, here's a screenshot that I took of the issue earlier (appologies for not uploading sooner)...

Hi, I'll try to tackle the add button styling, based on #70 reference =)
(mentored sprint in Vienna)

Here I attach a patch that @dimensional_dan and me have done toghether. We have changed the Add button to be more similar to the Popovers that propose the style guide.

The style guide propose that on hover we should have the bold text, but we had problems with the width of the popup, so we have kept the same font-weight.

It still need some work since the link should support the whole box instead of just the a tag.

Here is the screenshot to see how it looks now:

I made some changes to the look and feel of the add button and the "changed links" in the nav.

- The whole line in the dropdown is now clickable.
- Only the active item has a border.

The CSS should be completely refactored, it's a real pain to work inside it... Where is the Living Styleguide Initiative? I should start that. :)

Reroll of #79, just a small conflict on core/modules/views_ui/css/views_ui.admin.theme.css.

Thanks for the updates. I tested and the functionality worked as expected and it looks good except things go wonky temporarily when choosing a new option or switching tabs (though maybe that is reported already in another issue).

I reviewed the code for formatting and consistency only as I'm not a CSS expert.

1. +++ b/core/modules/views_ui/css/views_ui.admin.theme.css
   @@ -289,14 +289,8 @@ td.group-title {
   +  border-bottom: 1px solid #A6A6A6;
   

   Nitpick: Other colors are lowercase but this uses capitals... ?

2. +++ b/core/modules/views_ui/css/views_ui.admin.theme.css
   @@ -693,28 +653,15 @@ td.group-title {
   +#edit-displays-live-preview{
   

   Nitpick: Missing space before {.

3. +++ b/core/themes/seven/css/components/views-ui.css
   @@ -166,6 +154,24 @@ details.fieldset-no-legend {
   +  border-bottom: 1px solid #A6A6A6;
   

   Nitpick: Same as above regarding capitals.

> Use the visual appearance of standard tabs for displays:

I think this whole thing is a bad idea, sorry.

Things that *look* the same in a UI should *behave* the same.

The View display tabs don't behave like normal tabs:

- they switch with JS rather than reload the page
- they have an indicator for unsaved content
- the 'add' tab I can see in the screenshots people have uploaded for the patch has a dropdown.

I agree with Joachim.
On other pages the secondary tabs ( = local tasks) are separate pages. When users have unsaved changes and move to another tab then those changes get lost.

With this patch the different displays looks like tabs but aren't local tasks, and when the user moves from one to the other the unsaved changes are kept. By introducing this, we would teach users that it's okay to click away from a tab with unsaved changes - except that on all other tabs it's not and the user would loose their unsaved configuration.

Users might also start expecting an "Add something" functionality in the tabs on other pages where it's located somewhere differently (for example on Manage display pages.

At the moment this confusion doesn't exist because the button like display of different displays looks different enough from the local task tabs, but if we make them look to similar to make them look "good" then we reduce the usability.

Blocks-Layouts initiative is also temporarily abusing local tasks (see: #2922033-82: Use the Layout Builder for EntityViewDisplays), but the plan is to go with #2936655: Layout Builder should use the toolbar modes system once it exists. (also see: #2917777: Improvements to the styling, grouping, etc. of the Layout Builder UI actions form), could that be of use here too, or will it fracture UX even further ?
There's also #1791864: Add horizontal tabs support in core but it's empty.

Only patch reroll.

Setting Back to Needs Review.

This needs to be tagged for the usability team to review.

I maintain that it's a terrible idea and will make the Views UI very confusing to use.

The Views admin UI could probably do with some work, but this is the wrong direction. If we want to bring it more in line with core's UI patterns (which IS a good idea!) then vertical tabs would be a better core UI pattern to use here, because that pattern is for 'switching between different things before you've saved any of them'.

But then it would have the problem that it's taking up width on the page, and for the Views admin UI, that's at a premium.

My main problem with this is that I think the Problem described in the issue summary is not correct.

Views UI uses its own custom styles for displays as tabs instead of using either of the built-in standard ways.

The different displays of a View are all part of one page - one configuration that has a single Save button. The displays just make sure that not all is displayed on the page at the same time, to keep things manageable.

Different tabs and secondary tabs on other admin pages are actually different pages, that are grouped together, but that still stay separate and need to be saved separately.

It looks like work on Claro is taking this up [#https://www.drupal.org/node/3051605]

Hi,

I try on drupal-8.8.x-dev with Chrome Version 75.0.3770.100 (Build officiel) (64 bits) and Firefox 67.0.2 (64 bits) and your CSS is ok.
But as @Kristen Pol mention they are a glitch want you change from one type of the view to an over but it is append also without the patch.
I put a gif of the display glitch and a screenshoot of the glitch.

Removing "novice" issue as this is a bit meaty.

As a maintainer of Views on D7 I concur with #83 and #93 - the UI might not be the best, but its workflow and purpose doesn't fit with the UX pattern normally used by local actions.

I like that there is an effort to modernise the look of Views UI tabs and make them feel more consistent with the rest of Drupal, however I also agree with @DamienMcKenna in #97.

In #91 @joachim said:

The Views admin UI could probably do with some work, but this is the wrong direction. If we want to bring it more in line with core's UI patterns (which IS a good idea!) then vertical tabs would be a better core UI pattern to use here, because that pattern is for 'switching between different things before you've saved any of them'.

But then it would have the problem that it's taking up width on the page, and for the Views admin UI, that's at a premium.

I agree with that, if we want Views UI to look more consistent with the rest of Core then Vertical Tabs are the right direction here. Of course the horizontal space that they take up is a concern, but maybe that could work if the layout of the interface was reworked slightly.