Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UX] Add human name to fields list at admin/reports/fields #3316

Closed
jenlampton opened this issue Oct 16, 2018 · 16 comments
Closed

[UX] Add human name to fields list at admin/reports/fields #3316

jenlampton opened this issue Oct 16, 2018 · 16 comments

Comments

@jenlampton
Copy link
Member

jenlampton commented Oct 16, 2018

I often struggle finding the field I need on the fields list, because over time the machine name sometimes strays from the human readable label. I think we should add a column to the field list to include the human-readable label.

Here's an example of a site that's been upgraded from Drupal 6:
screen shot 2018-10-16 at 1 50 59 pm


PR by @klonos: backdrop/backdrop#2324
PR by @opi: backdrop/backdrop#2528 (based on klonos' 2324 with table sorting)

@klonos klonos changed the title [UX] add human name to Fields list at admin/reports/fields [UX] Add human name to fields list at admin/reports/fields Oct 17, 2018
@klonos klonos added this to the 1.11.2 milestone Oct 17, 2018
@klonos
Copy link
Member

klonos commented Oct 17, 2018

When the same field is reused across multiple content types (bundles in general), it might have different labels. So a single label per field instance is not applicable in all cases. I have filed a PR that does the following:

  • Splits the "(module: xyz)" to a column of its own (better readability, and we could allow sorting in the future)
  • Adds the priority-low class to the new "Module" column, so that it is hidden on tablet (leaving the class of the "Field type" column as priority-medium so that it is hidden only on mobile).
  • Renders the items in the "Used in" column as a list.
  • Appends "(field label: xyz)" at the end of the bundle name/link

Here's what it looks like:

screen shot 2018-10-17 at 8 08 19 pm

@klonos
Copy link
Member

klonos commented Oct 17, 2018

...while working on this ticket, I have noticed the following:

@olafgrabienski
Copy link

olafgrabienski commented Oct 17, 2018

When the same field is reused across multiple content types (bundles in general), it might have different labels. So a single label per field instance is not applicable in all cases.

Good point!

  • Appends "(field label: xyz)" at the end of the bundle name/link

I like the solution but I'm not sure if the prefix "field label" is necessary. To be less verbose, I'd suggest to drop it or to write just "label". Let's compare:

body | Text (long with summary) | Text | Layout Node (field label: Teaser)

body | Text (long with summary) | Text | Layout Node (label: Teaser)

body | Text (long with summary) | Text | Layout Node (Teaser)

@klonos
Copy link
Member

klonos commented Oct 17, 2018

Thanks for the review @olafgrabienski 👍

...yeah, I also initially thought to use just label:, but having that next to the content type label seemed confusing from a new Drupal/Backdrop user point of view (implies that the label is for the content type). No prefix at all is even worse if you ask me.

Having said that, I am OK to changing this to whatever works best, but lets wait for more feedback before changing anything.

@klonos
Copy link
Member

klonos commented Oct 17, 2018

...another idea is to have the entries in that column use a more "human" language. I have updated the PR:

  • "Used in" column renamed to "Used as"
  • entries use the format [field name] in [bundle] ("Teaser in Layout node" for example)
  • [field name] is rendered in italics and is now a link to the configure form of that field in the respective bundle.
  • the previous version of the PR was not using t() for the "field label:" string, so it would not have been possible to translate it. I now have used t() properly.

Here's how it looks:

screen shot 2018-10-18 at 5 48 48 am

Feel free to give it another spin @olafgrabienski 😉

@jenlampton jenlampton modified the milestones: 1.11.2, 1.11.3 Oct 18, 2018
@olafgrabienski
Copy link

entries use the format [field name] in [bundle]

Great idea, I like it! (Also the other suggestions.) I've had a quick look at the sandbox site, looks good!

@opi
Copy link

opi commented Oct 21, 2018

Nice ! I wonder about the use of italic, but looks good otherwise 👍

@klonos
Copy link
Member

klonos commented Oct 21, 2018

Thanks for the review guys 👍

@opi if you check the comment I have left in my PR, you will see that I am referring to a condition that checks for $admin_path, which was there before my PR. This either renders the field/bundle names as links, or as plain text (I am not sure what is the use case when this can happen). Now when mentioning names of things in help text or elsewhere using t(), we are usually rendering them by using a % placeholder instead if @, which makes them render in italics. We do this in /admin/structure/views for example, in the "View info" column where we render the various display names in italics. Also in various messages in the form of "The [thingy_name] [thingy] has been saved", like this for example:

screen shot 2018-10-22 at 4 22 53 am

Anyway, I am not adamant about it; we can remove the italics.

PS: I am working on updating the PR to allow all columns besides the "Used as" one as sortable (90% there). Please keep the reviews of the current PR coming, if there is anything that you thing I should change.

@klonos
Copy link
Member

klonos commented Mar 1, 2019

I have given the table sorting another go, but cannot get it to work. I have pushed my code in the PR, which does render the column headers as (seemingly) sortable, but the actual sorting is not happening. Can I please get a second pair of eyes to figure out what I might be doing wrong?

I have looked in the documentation and comments available at https://api.drupal.org/api/drupal/includes%21theme.inc/function/theme_table/7.x and also related sources in the internetz, like https://drupal.stackexchange.com/questions/14889/can-tablesort-be-used-without-a-query but cannot figure out what I'm missing.

@jenlampton
Copy link
Member Author

jenlampton commented Mar 8, 2019

Doesn't table sorting use database sorting? If these aren't in the database anymore (these are all config files now, right?), tablesort won't work here.

@opi
Copy link

opi commented Mar 9, 2019

@klonos here's a PR with table sorting : backdrop/backdrop#2528

@opi opi removed the help wanted label Mar 11, 2019
@klonos
Copy link
Member

klonos commented Mar 24, 2019

Thanks @opi ...RTBC 👍

PS: I did leave a small comment re a tiny coding standard fix (missing period at the end of a comment), but that should not stop this from RTBC 😉

@opi
Copy link

opi commented Mar 24, 2019

PS: Commited in my browser, thanks to Github awesome UI !

@klonos
Copy link
Member

klonos commented Mar 24, 2019

thanks to Github awesome UI !

Yeah, I really love that too @opi 😄

This is truly RTBC now.

@klonos
Copy link
Member

klonos commented Mar 25, 2019

...if #3620 gets accepted, we should revise the PR here to use that 😉

@quicksketch
Copy link
Member

Merged backdrop/backdrop#2528 into 1.x and 1.12.x. Thanks @klonos and @opi! I wasn't sure about the tablesort usage, it did seem a little out of ordinary but it worked well in practice in my testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants