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.
Comment | File | Size | Author |
---|---|---|---|
#28 | dropbutton_with_page.png | 13.54 KB | xjm |
#28 | dropbutton_links_no_path.png | 12.41 KB | xjm |
#26 | drupal-1831696-26.patch | 3.79 KB | dawehner |
#26 | interdiff.txt | 1.09 KB | dawehner |
#23 | 1831696-22.patch | 2.7 KB | damiankloip |
Comments
Comment #1
dawehnerIt doesn't really make sense to disable the button, because the dropbutton exposed other options,
so what about just hiding this part?
Comment #2
Bojhan CreditAttribution: Bojhan commentedI don't think the links should be gray? they should remain blue?
Comment #3
lisarex CreditAttribution: lisarex commentedI 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.
Comment #4
dawehnerNo 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.
Comment #5
damiankloip CreditAttribution: damiankloip commentedYeah, 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.
Comment #6
damiankloip CreditAttribution: damiankloip commentedSorry, amend to the above, I get it now, We mean before the view has ever been saved! That's not a bad point..
Comment #7
Bojhan CreditAttribution: Bojhan commentedSounds good to me too.
Comment #8
dawehnerThis is a good point!
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.
Comment #9
lisarex CreditAttribution: lisarex commentedCan you post a screenshot?
Comment #10
damiankloip CreditAttribution: damiankloip commentedPatch in #8 seems good to me.
Comment #11
lisarex CreditAttribution: lisarex commentedI'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.
Comment #12
damiankloip CreditAttribution: damiankloip commentedWe 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).
Comment #13
dawehnerAwesome!
Comment #14
webchickScreenshot please. :)
Comment #15
dawehnerAdd novice tag to make a screenshot.
Comment #16
mitron CreditAttribution: mitron commented#12: 1831696-12.patch queued for re-testing.
Comment #17
mitron CreditAttribution: mitron commentedView does not apply to latest pull of 8.x. Requeued for testing to confirm problem.
Comment #19
dawehnerRerole against.
Comment #20
mitron CreditAttribution: mitron commentedAttempt 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.
Comment #21
mitron CreditAttribution: mitron commentedComment #22
dawehnerGood observation!
Comment #23
damiankloip CreditAttribution: damiankloip commentedRight you are, this displayHandlers have changed.
Comment #24
dawehnerDamian's version is a bit better, so let's RTBC it (as always it should be green).
Comment #25
catchWorth having a test for?
Comment #26
dawehnerYeah why not.
Comment #27
Bojhan CreditAttribution: Bojhan commentedComment #28
xjmThis 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:
Path is not saved:
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).
Comment #29
xjmComment #30
tim.plunkettI'm reasonably sure that those aren't links. They're buttons. Must have lost some CSS somewhere.
Comment #31
xjmOver in #1965910: Remove enable/disable entity operations Xano is calling them links.
Comment #32
tim.plunkettThe 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.
Comment #33
alexpottCommitted 763caef and pushed to 8.x. Thanks