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

Comments

damiankloip’s picture

damiankloip commented
Status: Active » Needs review
StatusFileSize
new 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
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 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 commented
Issue tags: +Usability, +BADCamp2012UX
damiankloip’s picture

damiankloip commented
StatusFileSize
new 1831142-5.patch744 bytes

Sounds good to me!

damiankloip’s picture

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

dawehner
Primary language German
commented
StatusFileSize
new after.png22.56 KB
new before.png7.66 KB

Fine, here is one.

Bojhan’s picture

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

:)

lisarex’s picture

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
commented

Is this worth adding a test for per #2?

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new drupal-1831142-11.patch2.1 KB

Let's do it, you never know.

catch’s picture

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

Committed/pushed to 8.x, thanks!

k.skarlatos’s picture

k.skarlatos commented

Will this get ported to 7.x?

Bojhan’s picture

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 commented
Status: Fixed » Active
dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +Novice

Good point! That is nothing really complicated to implement.

mducharme’s picture

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

Drupal 7 backport ready for review.

dawehner’s picture

dawehner
Primary language German
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 commented
StatusFileSize
new 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 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
commented
Status: Reviewed & tested by the community » Fixed

Awesome! Committed and pushed

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