According to the UI patterns, mandatory fields shouldn't be part of a vertical fieldset.

Suggested change. Move the general tab out of the the fieldset.

CommentFileSizeAuthor
#4 general_vertical_tabs.patch2.9 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

With #1099406: Convert role permissions settings into permissions, we could even drop the vertical tabs thing completely, we'll see.

BenK’s picture

Subscribing

Berdir’s picture

Title: Reviste usage of vertical_tabs in relationship type form » Revisit usage of vertical_tabs in relationship type form

Updated title.

Berdir’s picture

Status: Active » Needs review
FileSize
2.9 KB

Here is a first patch. If you combine this with the permissions patch (they might not apply both, though), there is only a single vertical_tab remaining....

Berdir’s picture

On the other side, privatemsg integration will likely add another one..

BenK’s picture

Status: Needs review » Needs work

Hmmm...

The patch applies fine, but I'm not sure this is a visual improvement. The vertical tabs are pushed pretty far down the page now because the settings outside the tab are pretty long.

One possibility would be to just have the name and plural name outside the vertical tabs and then create tabs for the approval and one-way stuff. Maybe call them "Approval" and "Direction", respectively? Or think of a name that combine them into one?

Also, would Private Message integration actually add a tab on this page or would those permission-related settings appear on the Permissions page?

--Ben

Berdir’s picture

Not sure, if you have a look at the add/edit node forms and Configuration > People > Account settings, it's explicitly far down too.

I think all these settings are something that you *need* to think about when creating an relationship so it makes IMHO sense to have them outside.

Yes, I think #1108520: Private message integration: Sending to one-way relationship types (differentiate between relationship direction) will add options, will also comment there about further ideas I have.

BenK’s picture

Status: Needs work » Reviewed & tested by the community

Okay, you've convinced me! ;-) Also, it's not as far down on the page as I thought because we're using #states now.

I tested the patch again and everything is working well, so this is RTBC.

--Ben

Berdir’s picture

Status: Reviewed & tested by the community » Fixed

Ok, commited.

Status: Fixed » Closed (fixed)

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