Problem/Motivation

Field labels are not unique which can make managing form and view displays challenging if multiple fields have the same label.

There are usually two workarounds for now:

  • Open the console to check the machine name of a given table row
  • Open another browser tab to double check fields by their label and machine name

Steps to reproduce

  • Enable the Field UI module
  • Create two fields on any given content entity and give them the same label
    example: Go to Admin > Config > Account > Parameters and create a new field_phone field labelled Phone and a second field_cellphone labelled Phone
  • Go to the "Manage form display" or "Manage display"
  • Try to determine which row corresponds to which field
    Result: site builder can't tell which field is which
    Expectation: site builder can identify each field easily by its unique machine_name

Proposed resolution

Show the machine_name of each field in the table rows of "Manage form" and "Manage display" on all content entities.

Remaining tasks

  1. Add the field in the Manage display table
  2. Add functional tests
  3. Code review
  4. Make a Usability review

User interface changes

There is a new column right to the first "Field" column (see capture below).

API changes

None.

Data model changes

None.

Release notes snippet

TBD

Issue fork drupal-3306820

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

kulmjens created an issue. See original summary.

kulmjens’s picture

cilefen’s picture

Status: Active » Needs review
kulmjens’s picture

Fixed the DCS problem with the missing blank.

christyanpaim’s picture

Status: Needs review » Reviewed & tested by the community

Works fine to me!

sascha_meissner’s picture

applied the patch and it works as expected, find this very useful

quietone’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update, +Needs usability review, +Needs tests

@christyanpaim, Welcome to Drupal! There are many steps needs to be done before a change can be RTBC. A place to start is the Contributor tasks, there is one for the Review a patch or merge request process.

I am restoring the remainder of the standard issue template so we can keep track of the work to be done on this issue. Tagging for an update though for the sections to be completed.

This is making changes to the UI so it needs to pass the Usability core gate, adding tag. Tests are needed, adding tag. Once there is direction supplied from the good folks in #ux and a working patch, this will need before and after screenshots in the Issue Summary.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

matthieuscarset made their first commit to this issue’s fork.

matthieuscarset’s picture

Status: Needs work » Needs review

To keep consistency between all Field UI tabs, I suggest we add the Machine name column to Manage form and Manage display tables.

I have not found any existing tests related to table headers or table rows' columns thus I have not added new tests. I guess it is not even necessary to check if this column exists 🤷. If so, please feel free to remove the "Need tests" tag from this issue. If not and we want tests, please help find where I should add such a test.

See attached MR !7612

matthieuscarset’s picture

StatusFileSize
new93.42 KB

Capture of the Manage form display with the new Machine name column:

smustgrave’s picture

Status: Needs review » Needs work

Was previously tagged for issue summary update which will need to happen.

As for the tests ManageDisplayTest.php seems like a perfect spot to me. Don't see a test case that perfectly matches so probably could just start a new one.

matthieuscarset’s picture

Assigned: kulmjens » matthieuscarset
matthieuscarset’s picture

Assigned: matthieuscarset » Unassigned
Issue tags: -Needs tests

Removing "Need tests" tag as relevant test was added to ManageDisplayTest.php.

Pending code review, issue summary update and usability review

matthieuscarset’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update
matthieuscarset’s picture

Status: Needs work » Needs review

Issue summary updated.

Marking as NR for Usability and Core reviews.

bessonweb’s picture

It's a good idea!

+1 for me.

bernardm28’s picture

It's an interesting idea and would save some time as one starts theming an entity.
Does it need to be there all the time? Maybe not, but it would be great to have it as a development option or have the ability to unhide it from somewhere on that table. Kind of hidden in plain site per se.

dww’s picture

Status: Needs review » Needs work

Love this. Added a note about test efficiency to the MR. I don't think we need a whole new test (and therefore, complete install of Drupal) for 2 assertions. 😅 Let's move these into an existing test to save time / CO2 / DA $.

Otherwise, seems like a win to me, and probably very close to RTBC.

Thanks!
-Derek

dww’s picture

Status: Needs work » Needs review

Looking at the rest of core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php makes me cringe. Wow that's a lot of separate (tiny) test methods. 😢 Obviously out of scope to refactor all of this, but probably worth a follow-up.

That said, I'm not sure what I'd suggest for this issue, so maybe adding a new test method is fine. Restoring NR, but it smells a little fishy to me. 😅

smustgrave’s picture

Status: Needs review » Needs work

Hiding the patches

Left some comments. Would agree that this could be a follow up for refactoring but think a good task would be to repurpose testSingleViewMode() into testManagerDisplaysInterface or something. Take the comment of the test now "Tests hiding the view modes fieldset when there's only one available." and make it a comment in the test. And obviously update the existing test description.

matthieuscarset’s picture

Status: Needs work » Needs review

Thank you for your reviews. I've updated the tests/assertions. Hoping it is acceptable now.

Marking this issue as NR.

smustgrave’s picture

Applied a small name change to the test.

Rest seems fine to me so +1 but will still need usability.

bernardm28’s picture

StatusFileSize
new15.33 KB

What if Instead of having it's own column it displays as an html subscript ?
image
Idk if it needs it's own column since it could be attached under the label for the field.

smustgrave’s picture

Like the idea as we know the machine name will be limited length. May want to wait for UX though. Posted in #ux in slack if anyone frees up

simohell’s picture

Having a separate column makes it easier to visually scan labels and machine names. I suppose there are pros and cons to both ways.

rkoller’s picture

Issue summary: View changes
Issue tags: -Needs usability review

Usability review

We discussed this issue at #3441951: Drupal Usability Meeting 2024-04-26. That issue will have a link to a recording of the meeting.

For the record, the attendees at the usability meeting were @benjifisher, @BlackBamboo, @rkoller, @skaught, and @worldlinemine.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

In general there was a clear consensus that it is convenient to have the machine name right in place. We've considered the point raised in a corresponding discussion in the #frontend channel on the Drupal slack that the Manage fields page is already providing the machine names. But on the other hand on the Manage form display page as well as the Manage display page you not only have the machine names for fields but also those for base fields. The machine names for base fields are harder to obtain, requiring turning on the debugging and using some sort of dump() function according to @mariohernandez. With the MR in place the machine names for base fields are available as well. So you don't have to switch in-between tabs/pages or open a second browser window anymore.

We've also considered the suggestion raised by @bernardm28 in #28 moving the machine name from a dedicated column to the label column and display it as an html subscript (plus some aria as he added on the corresponding thread on Slack). That might save one column and horizontal screen real estate but on the other hand moving the machine name as a subscript underneath the label might entail other problems. The subscript would have to be labeled (not only by aria for screenreader users but also for sighted users) and just moving vertically through a column would announce the label and the machine name in a row for each cell. If a screenreader user just wants to get the machine names announced that would be difficult, since the label would always be prepended. So having a dedicated column for machine names requires a bit more horizontal screen real estate, but it is clearer, easier scan-able and less error-prone in particular for screen reader users. Each column is holding specific information and the user is able to navigate by keyboard deliberately retrieving the desired information.

We've also taken a look on mobile view ports in combination with extra long labels and machine names. The machine names are getting wrapped same as labels.

So overall we have no concerns with the MR and it looks good to go from our perspective. I'll remove the needs usability review tag again. The only thing I am unsure about is which status should be set. Since the code review task is not crossed of yet i'll leave the status at needs review.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Thanks @rkoller and usability team.

All the feedback on the MR has been addressed and is functioning. Test coverage is also there. Believe this is good to go.

xjm credited benjifisher.

xjm credited BlackBamboo.

xjm credited SKAUGHT.

xjm credited worldlinemine.

xjm’s picture

Crediting usability meeting participants.

xjm’s picture

Issue tags: +Portland2024

Saving other credits as well.

quietone’s picture

Status: Reviewed & tested by the community » Needs work

There is a comment that should be changes to be correct. Setting to needs work.

The issue summary has an open item for needing a code review. This was pointed out in #28 as well. Has there been a code review?

matthieuscarset’s picture

Status: Needs work » Reviewed & tested by the community
longwave’s picture

Status: Reviewed & tested by the community » Needs work

Tests are failing.

As a usability fix and UI change this is too late for 10.3.0 now we are in RC, but it is still eligible for the beta phase of 11.0 and also for 10.4.

matthieuscarset’s picture

Status: Needs work » Needs review

I don't think failures are related to this code.

All errors seem to be related to something broken in Umami.

Example of a current failure: https://git.drupalcode.org/issue/drupal-3306820/-/jobs/1721378

It's a shame to miss a minor version 10.3 because of an unrelated broken part 😓.

How should I proceed to move this issue along?

smustgrave’s picture

Recommend rebasing

matthieuscarset’s picture

Sorry I'm not sure.

Rebasing `issue/drupal-3306820:3306820-show-machine-name` over `11.x` ?

This gives a 2k files conflict.

smustgrave’s picture

Need to rebase with origin/11.x

Current MR is 300+ commits behind which may address the failures

smustgrave’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Rebased seemed to address the unrelated failures.

Test name appears to be updated.

Code changes make sense for the task being updated.

xjm’s picture

As a usability fix and UI change this is too late for 10.3.0 now we are in RC, but it is still eligible for the beta phase of 11.0 and also for 10.4.

Actually, it's a UI change, so not beta-eligible for 11.0. Also, UI changes aren't eligible for maintenance minors, which 10.4 is. So I think this is an 11.1 change.

xjm’s picture

Status: Reviewed & tested by the community » Needs work

I manually tested this and noticed something that I think needs work:

When you compress the screen to a smaller size, before it hits the breakpoint where the new column would be hidden, the machine names start to wrap, which is something machine names shouldn't do.... but what's worse, it automatically hyphenates the machine names, which is definitely way confusing, because it makes you think the hyphens are a part of the machine name.

So, I think we need to change the word-break or word-wrap behavior for the column, and ensure the column is hidden when some reasonable length of machine name does not fit, and that exceptionally long machine names are elided somehow (which I vaguely remember seeing elsewhere in other issues). To suggest a solution, I went to the Manage fields page... and discovered it has the same problem. So, I filed #3455155: Fix word-wrapping behavior of machine names in UI tables.

xjm’s picture

Title: Show machine name in "manage form display" and "manage display" table row » Show machine name in "Manage form display" and "Manage display" table row
Status: Needs work » Reviewed & tested by the community

Did not mean to NW since I filed the followup myself.

  • xjm committed e091a2fd on 11.x
    Issue #3306820 by matthieuscarset, kulmjens, smustgrave, bernardm28, xjm...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 11.x. Thanks!

quietone’s picture

@xjm, thanks for making the followup. It saves me chasing up 'needs followup' for fixed issues.

nod_’s picture

Status: Fixed » Closed (fixed)

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

donquixote’s picture

I like this change. Thanks everybody!

Just some notes:

  • The columns gets even more tight when I click "Show row weights".
    I like to do this to manually set row weights to reduce noise in a git commit.
    But it is worth it imo.
  • This needs work in field_group and possibly other contrib modules. Currently it looks broken with field_group enabled.
    Again, that's ok with me.
    But it deserves a note in the "API changes".

From a usability perspective:
In all the projects I ever worked on, the site builder who manages displays with field_ui is the same person who will then run "drush config:export" and commit the changes in git.
There is no point in hiding machine names from this person.

donquixote’s picture

It could also be nice to have some kind of feature detection possibility, so that a module like field_group can support different versions.
Currently it could check if any field row has a cell key 'machine_name', but this is not 100% reliable if there are no fields for some reason.

xjm’s picture

Amending attribution.