This is a portion of #2900409: [Meta] Improve UI of Reference field settings form, separated out after comment #133.
There is a core UX concept called progressive disclosure, where you hide complexity until the point at which you actually need it. It would be good if the field settings form for Entity Reference fields used this concept.
Hide the Sort options select in the Entity Reference field settings dialog unless one or more bundles is selected.
None
Hide the Sort options select in the Entity Reference field settings dialog unless one or more bundles is selected.
Here is a screenshot before applying the patch. No bundles are selected, but the "Sort by" select list is visible:
After applying the patch, the select list is hidden when no bundles are selected:
None.
None.
Probably not necessary, a small UX improvement.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | 3089523-19.patch | 3.24 KB | harpreet16 |
| #6 | 3089523-6.patch | 3.21 KB | benjifisher |
| #6 | interdiff-3089523-2-6.txt | 1.06 KB | benjifisher |
| #3 | after-selected.png | 58.15 KB | benjifisher |
| #3 | after-none.png | 50.29 KB | benjifisher |
Comments
Comment #2
benjifisherI think this patch should do it.
Do we have the right component for this issue (Field UI module)? The changed behavior is under
core/lib/Drupal/Core/Entity, and the test is in the Field module. I think this is a symptom of a broader problem.Comment #3
benjifisherI am adding some screenshots to the issue description.
Comment #5
jhodgdonLooks like some additional test fixes are needed...
Comment #6
benjifisherFunny, the test passed locally (in the UI). Maybe this is the right way to check for visibility.
Comment #8
benjifisherThe first test of the patch in #6 failed on a different test. I triggered a retest. Since the retest passed, I am moving this issue back to NR.
Comment #9
jhodgdonWe should probably make sure there is a Random Test Failure issue about that failed test, as it is most likely a test that will fail sometimes due to timing -- at least the error looks like that could be the case (something not on the screen yet from Ajax I suspect). Let's see...
Hm... I guess it was probably actually an infrastructure glitch, because one of the errors in that test run was "Failed to start the web server". So I think we should let it go.
Anyway, the code, test, and screenshots look good to me. However, I did some manual testing, and ... oh never mind. I noticed #3089525: Sort options should correspond to bundles selected for entity reference field settings but that is a separate issue. So, this is working fine in manual testing: the Sort options list is hidden when no bundles are selected, and visible if at least one option is selected.
Comment #10
benjifisherThanks for looking into the test failure. I know I should have done that, but I did not have time/patience/energy.
Comment #11
benjifisherOnce again, I left an issue open in a browser tab, forgot to force-reload, and accidentally changed the issue status.
Back to RTBC as in #9.
Comment #14
lauriiiTested this manually and everything seemed fine. Also double-checked that this doesn't cause any regressions for non-JavaScript users and as expected, everything worked fine.
Committed 7553451 and pushed to 9.0.x and 8.9.x. Thanks!
Comment #15
lauriiiComment #16
jhodgdonSetting status to "To be ported" like other parts of #2900409: [Meta] Improve UI of Reference field settings form to see if we want it in 8.8 too.
Comment #17
lauriiiThis doesn't feel very risky but I guess the biggest risk I can think of is some projects tests breaking. I would love to hear what other people think about backporting this.
Comment #18
alisonBackporting would be sweet, if it's practical for folks to implement!
Comment #19
harpreet16 commentedNote: This patch is applicable for 8.8.x version after https://www.drupal.org/project/drupal/issues/3089521 is applied.
@lauriii pls let me know if any other update is needed. Tested on local setup. Works fine for me.
Comment #20
harpreet16 commentedComment #21
harpreet16 commentedComment #22
lauriiiIt's getting late for us to backport this to 8.8.x. Marking this as fixed since this has been committed to 8.9.x already.