So currently we have this code determining what is displayed for an empty path in the edit UI.

  $path = strip_tags('/' . $this->getOption('path'));

  if (empty($path)) {
    $path = t('None');
  }

Because a '/' is always appended to the path, it's never empty. So for an empty path we are just left with the '/' character.

CommentFileSizeAuthor
#19 views-path-change-1831142-19.patch993 bytesmducharme
#17 views-path-change-1831142-17.patch1000 bytesmducharme
#11 drupal-1831142-11.patch2.1 KBdawehner
#7 before.png7.66 KBdawehner
#7 after.png22.56 KBdawehner
#5 1831142-5.patch744 bytesdamiankloip
#1 views-empty-path.patch745 bytesdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Active » Needs review
FileSize
views-empty-path.patch745 bytes

Here is a patch with the text changed to 'No path defined', based on the Views usability study feedback.

We should probably add a test for this too.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

This looks perfect

I guess screenshots would show how great this actual improvement is.

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented
Status: Reviewed & tested by the community » Needs review

Yhea, this is a bit of an unwritten rule. But we generally really like to have screens before UX improvements are RTBC, just to provide that last check.

I would love if we can actually not use "defined", and just say "No path is set"

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented
Issue tags: +Usability, +BADCamp2012UX
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
1831142-5.patch744 bytes

Sounds good to me!

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Title: path is never empty in option summary. » Path is never empty in option summary.
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
after.png22.56 KB
before.png7.66 KB

Fine, here is one.

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented
Status: Needs review » Reviewed & tested by the community

:)

lisarex’s picture

lisarex CreditAttribution: lisarex commented

Too late now, but +1 from me. This came up in 2 of my sessions. Plus '/' is a tiny target area!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Is this worth adding a test for per #2?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
drupal-1831142-11.patch2.1 KB

Let's do it, you never know.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

k.skarlatos’s picture

k.skarlatos CreditAttribution: k.skarlatos commented

Will this get ported to 7.x?

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 7.x-3.x-dev
Component: views_ui.module » User interface

No reason it cant

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented
Status: Fixed » Active
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +Novice

Good point! That is nothing really complicated to implement.

mducharme’s picture

mducharme CreditAttribution: mducharme commented
Status: Active » Needs review
FileSize
views-path-change-1831142-17.patch1000 bytes

Drupal 7 backport ready for review.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Let's not backport the test unless someone is willing to take the effort.

+++ b/plugins/views_plugin_display_page.incundefined
@@ -251,9 +251,13 @@ class views_plugin_display_page extends views_plugin_display {
+    ¶

We shouldn't introduce that whitespace here.

mducharme’s picture

mducharme CreditAttribution: mducharme commented
FileSize
views-path-change-1831142-19.patch993 bytes

Thanks for spotting that. I've updated the patch to remove the extra whitespace. Patch applies without warning now.

k.skarlatos’s picture

k.skarlatos CreditAttribution: k.skarlatos commented

Thanks. Its seems to work ok for me, it applies and works fine without any warnings or errors.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Reviewed & tested by the community » Fixed

Awesome! Committed and pushed

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