Problem/Motivation

During testing we found a problem when people clicked the "view page link".

‘View page’ link goes to the home if the path isn't set, causing confusion because people think that is what they created when in fact its not.

Proposed resolution

This is an interesting challenge, because we basically create a link that instead of helping people picture what its about. I don't necessarily want to create a solution where we provide a disabled button just to inform the user, because disabled buttons make people go crazy.

Instead I propose we only show it when a path is filled out and its saved. For the wizard workflow this is easy, for the non-wizard workflow it might be a little trickier.

User interface changes

Remove view title when there is no path set.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
936 bytes
7.84 KB
8.04 KB

It doesn't really make sense to disable the button, because the dropbutton exposed other options,
so what about just hiding this part?

Bojhan’s picture

I don't think the links should be gray? they should remain blue?

lisarex’s picture

I am slightly concerned that the Views pros will click 'clone page' out of habit and become annoyed. Could you replace 'view page' on unsaved views with 'save and view page'.

I agree that the links should remain blue.

dawehner’s picture

I don't think the links should be gray? they should remain blue?

No idea, but this seems to be another bug and not related to this issue, sorry, i guess #1826574: Move Views theme-specific CSS to their respective themes caused that.

damiankloip’s picture

FileSize
2.47 KB

Yeah, also I think some other css was removed from core recently. I'm not sure it's anything we have done in views.

I'm not sure about the 'save and view page' as you don't really have to save the view when leaving the page.

How about we change the order to be more logical? So the order (for enabled) is disable, clone, delete. To me this makes the most sense operationally. I have also added a check to see if the view is enabled before showing the 'view page' link, as there is no point in having this if a view is not enabled.

damiankloip’s picture

Sorry, amend to the above, I get it now, We mean before the view has ever been saved! That's not a bad point..

Bojhan’s picture

Sounds good to me too.

dawehner’s picture

FileSize
2.79 KB
+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.phpundefined
@@ -378,9 +378,9 @@ public function getDisplayDetails($view, $display) {
+        elseif ($view->isEnabled() && $view->get('executable')->displayHandlers[$display['id']]->hasPath()) {

This is a good point!

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.phpundefined
@@ -391,6 +391,16 @@ public function getDisplayDetails($view, $display) {
+        if ($is_enabled) {
+          $build['top']['actions']['disable'] = array(

From a user perspective it seems to be for me that cloning or deleting a display is used way more often then disabling a display. You either want to have a display or get rid of it.

This patch also hides the link as long the view is not saved yet.

lisarex’s picture

Can you post a screenshot?

damiankloip’s picture

Patch in #8 seems good to me.

lisarex’s picture

I've applied this patch and I think having 'clone' as the default is better, although if they're moving fast, they'll create a clone (or 2) of their page without meaning to.

@ damienkloip, in comment #6 are you saying that the suggestion for the link to be 'save and view page' when the view hasn't been saved is a good one? I don't know what it would take to achieve this, but I'd be interested in testing it out.

damiankloip’s picture

FileSize
1.13 KB
2.77 KB

We can remove the check if the view is saved or not since views are all saved when they are created now (#1848836: Change Views Wizard's primary button to 'Save and edit' and remove 'Save & exit' button).

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!

webchick’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs screenshots

Screenshot please. :)

dawehner’s picture

Issue tags: +Novice

Add novice tag to make a screenshot.

mitron’s picture

#12: 1831696-12.patch queued for re-testing.

mitron’s picture

View does not apply to latest pull of 8.x. Requeued for testing to confirm problem.

Status: Needs review » Needs work

The last submitted patch, 1831696-12.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
2.71 KB

Rerole against.

mitron’s picture

Status: Needs work » Needs review

Attempt to create screen shot but following error after applying patch when clicking save & edit.

Fatal error: Cannot use object of type Drupal\views\DisplayBag as array in /Users/jimhollcraft/Documents/IdeaProjects/drupal8/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.php on line 388.

After removing patch, this error does not occur.

mitron’s picture

Status: Needs review » Needs work
dawehner’s picture

FileSize
997 bytes
2.72 KB

Good observation!

damiankloip’s picture

FileSize
981 bytes
2.7 KB

Right you are, this displayHandlers have changed.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Damian's version is a bit better, so let's RTBC it (as always it should be green).

catch’s picture

Status: Reviewed & tested by the community » Needs review

Worth having a test for?

dawehner’s picture

FileSize
1.09 KB
3.79 KB

Yeah why not.

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community
xjm’s picture

Category: task » bug
FileSize
12.41 KB
13.54 KB

This works, but testing it, I noticed something odd with the link styling. (This also occurs in HEAD, so not related to this patch.)

Path is saved:
dropbutton_with_page.png

Path is not saved:
dropbutton_links_no_path.png

Why are they different colors? Bad a:visited styling? Folllowup?

We should also file a followup to sentence-case these operations (and those in the dropbutton for the view).

xjm’s picture

tim.plunkett’s picture

I'm reasonably sure that those aren't links. They're buttons. Must have lost some CSS somewhere.

xjm’s picture

Over in #1965910: Remove enable/disable entity operations Xano is calling them links.

tim.plunkett’s picture

The enable/disable links for the View config entity are links.

Since these are part of the ViewEditFormController, these are for the *display* and are submit buttons.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 763caef and pushed to 8.x. Thanks

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