Problem/Motivation

Sometimes the amount of meta tags you need to edit on a node edit form is too big to make good UX to have in the sidebar, so you turn that option off and place it in a tab (could be done via field_group module). This however means that in order to access the meta tags you first have to click the tab and then open the details.

This is not a bug or a problem, but a feature request to allow for more flexible form layouts.

Steps to reproduce

  1. Add meta tags field to node
  2. Use field_group module to add tabs to node form
  3. Move meta tags field to its own tab and disable using sidebar
  4. Save the form display
  5. See that you now have to unnecessarily open the details on the tab

Proposed resolution

Add an option to use a normal container wrapper instead of a details container, this would be as simple as setting the element's #type to "container" instead of "details".

Remaining tasks

Review of patch.

User interface changes

Added widget option to use details container, defaulting to true to keep BC.

CommentFileSizeAuthor
#9 metatag-3170178-9.patch2.43 KBdionsj
#2 metatag-3170178-2.patch1.97 KBdionsj

Issue fork metatag-3170178

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:

About issue forks

Comments

dionsj created an issue. See original summary.

dionsj’s picture

dionsj
Primary language Danish
Location Århus
commented
StatusFileSize
new metatag-3170178-2.patch1.97 KB

Here is the patch for the proposed changes.

Status: Needs review » Needs work

The last submitted patch, 2: metatag-3170178-2.patch, failed testing. View results

dionsj’s picture

dionsj
Primary language Danish
Location Århus
commented
Status: Needs work » Needs review

Not sure the failed tests are the fault of my patch, as they all seem to be regarding a missing "Save field settings" button, which from a search through the module, doesn't exist as the string "Save field settings" is only referenced in the tests that are failing.

So I would assume that the failing tests are because of another issue.

Moving the issue status back to needs review, if people disagree feel free to change it back but please also explain how my patch causes the failing tests so I can fix it.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Closed (duplicate)
Parent issue: » #2844696: Allow the field to control whether it is displayed in the sidebar or not

This is already possible, the change was added in #2844696: Allow the field to control whether it is displayed in the sidebar or not.

dionsj’s picture

dionsj
Primary language Danish
Location Århus
commented
Status: Closed (duplicate) » Needs review

I disagree, that issue only relates to the sidebar, not the details wrapper itself, when not using the sidebar it still displays it as a details wrapper container, which is fine if you have it shown with other form elements, but if shown on it's own tab it makes less sense to have it use a details wrapper.

The patch I've attached adds an option to use a normal container wrapper instead of details, as per issue description.

I don't consider it a duplicate as the use case of the sidebar issue you mentioned isn't the same.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Ok, I see your point I think it needs a little improvement, e.g. how does it work if it's set to show in the "advanced" sidebar and the details option is disabled?

dionsj’s picture

dionsj
Primary language Danish
Location Århus
commented

It would display the contents of the details, which does indeed make it much less UX friendly, I'll add some form states so that the option to disable details isn't available unless it's not in the sidebar, as well as programmatically not change it to a container if it's in the sidebar. This seems like the best approach without messing with current behavior and keeping good UX in both situations.

dionsj’s picture

dionsj
Primary language Danish
Location Århus
commented
StatusFileSize
new metatag-3170178-9.patch2.43 KB

Attached is an updated patch, it does the following to improve UX:

  • If sidebar is enabled, the option (and summary) for whether details container is enabled is not shown
  • If sidebar is enabled, the details container option is always set to use details, even if you change it to false in the config

This should result in you being able to display the widget in the following ways:

  • In the sidebar as a collapsed details container (default behavior, sidebar: true)
  • In the normal form area as a collapsed details container (sidebar: false, details: true)
  • In the normal form area as a normal container (sidebar: false, details: false) <- this should enable you to add the widget to it's own tab or add it to a custom details wrapper, all of which can be done via the field_group module

Status: Needs review » Needs work

The last submitted patch, 9: metatag-3170178-9.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

dionsj’s picture

dionsj
Primary language Danish
Location Århus
commented
Status: Needs work » Needs review
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

The tests currently fail, so this needs work.

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

geekygnr opened merge request !28

geekygnr’s picture

geekygnr commented

Merged the patch into a pull request.

geekygnr’s picture

geekygnr commented

All of the errors regarding the missing button were caused by schema errors being triggered when trying to add a new field. I added the schema and the tests I checked locally were resolved. I am going to let the CI tools run the rest of the tests.

geekygnr’s picture

geekygnr commented
Status: Needs work » Reviewed & tested by the community

Most of my changes were small and test related. I have reviewed everything else and it looks good. I am going to mark this RTBC.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

This is almost perfect, but I think it would be useful to update the checkbox description to explain scenarios when it would be useful.

  • DamienMcKenna committed b1a5232 on 8.x-1.x authored by dionsj 
    Issue #3170178 by geekygnr, dionsj, DamienMcKenna: Allow widget to use...
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Fixed

Committed. Thank you.

Status: Fixed » Closed (fixed)

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