processwire / processwire-issues

ProcessWire issue reports.
45 stars 2 forks source link

"Blank" titles in InputfieldSelector Lister preview when viewing users #158

Closed adrianbj closed 1 year ago

adrianbj commented 7 years ago

Short description of the issue

If you set allowSystemCustomFields to true and use an InputfieldSelector inputfield to select users, the titles show up as "Blank"

Expected behavior

Should fallback to name

Optional: Suggestion for a possible fix

Change:

if(!strlen($value)) $value = $this->blankLabel;

to:

if(!strlen($value)) $value = $p->name;

https://github.com/processwire/processwire/blob/43986173fb449bf0fb967a0d613896f2e3d6b4da/wire/modules/Process/ProcessPageLister/ProcessPageLister.module#L1312

I think a fallback to the page name makes sense for all pages (not just user pages) - I'd rather see the name than "Blank"

kixe commented 7 years ago

If a title is not assigned it should be displayed as is (blank). There is similar no fallback from $page->title to $page->name If you want to see the name change the column settings of InputfieldSelector. I do not agree to have a fallback here like in the pagetree.

ryancramerdesign commented 7 years ago

In PageList, there's just that single label, so having a fallback makes sense. But in Lister, you've got multiple columns possible, and they are configurable. I think they should be accurate and consistent with the contents of the field being presented. So if something is blank, Lister should communicate that shouldn't it? Though for the first/actions column only, I suppose we could add some clarification, like Blank <small>(the-page-name)</small>?

adrianbj commented 7 years ago

I think perhaps it needs to be a little smarter than just returning the name as a fallback. Perhaps it should check if the template of the page has a title field. If it doesn't (eg the user template), then it should use the name without any "blank" label. If it does have a title field, then I agree that Blank <small>(the-page-name)</small> is a good idea.

ryancramerdesign commented 7 years ago

Since this is Lister and not PageList, it's got to be careful about falling back to anything (like a name) without being really clear that the value is indeed blank/not present, regardless of whether the field is applicable to the template or not. Otherwise it's showing a column heading with "title" and then showing a "name", which is false data, and certain to cause confusion. But I still think it would be fine to show the name so long as it's clear the value for the requested field shown in the column heading isn't there. After doing that, a <small> name seems useful and unlikely to confuse anyone.

kixe commented 7 years ago

The display of "Blank" in the column Title in case of user template is already confusing, due to the fact there is no title field assigned to the user template. Furthermore Processwire settings allows to create templates including a title field which is not required. I think there should be a more precice destinction between "Blank" and "not assigned".

I agree to provide helpful information about the listed page in case of missing title. Since the first column (even if not Title) always provide the edit link, It could be helpful to add a title attribute to this link holding the id and the name.

<a class="actions_toggle" href="#" title="id:41 name:admin" id="page41"><span class="PageListAccessOff">Blank</span></i></a>

netcarver commented 5 years ago

Adding labels to bump.

matjazpotocnik commented 1 year ago

@adrianbj @kixe is this still an issue? This is how the relevant code looks now:

https://github.com/processwire/processwire/blob/f1c20282f62f47b66bf24786f80e141e60f16ff4/wire/modules/Process/ProcessPageLister/ProcessPageLister.module#L1706-L1720

image

EDIT: added screenshot

matjazpotocnik commented 1 year ago

@adrianbj, @kixe, closing, reopen if needed.