the recent change to put the taxonomy pulldown into its own fieldset may have made sense from a UI perspective for some kinds of nodes, but it's certainly very confusing looking in the forums.

A drupal admin would understand that forums are actually implemented in terms of Categories, but a normal forum user will be confused by the box "Categories" around the forum selector.

CommentFileSizeAuthor
#13 taxonomy.module.diff.txt845 bytesdrumm
#11 removefieldset_2.patch1.28 KBnickl
#9 removefieldset_1_0.patch1.28 KBflk
#8 removefieldset.png11.04 KBnickl
#7 removefieldset_1.patch1.27 KBnickl
#6 removefieldset_0.patch1.27 KBnickl
#5 removefieldset-is_array.patch528 bytesbudda
#2 removefieldset.patch525 bytesbudda
Screenshot_2.png16.53 KBWesley Tanaka

Comments

moshe weitzman’s picture

moshe weitzman commented

indeed ... IMO, we should only use fieldset when there are 2 or more fields to be grouped

budda’s picture

budda
Primary language English
Location Cheshire
commented
Component: base system » taxonomy.module
Status: Active » Reviewed & tested by the community
StatusFileSize
new removefieldset.patch525 bytes

Agreed, it makes simple forms look more complicated than they should be.

Attached patch checks if there are multiple vocabularies in the form before enclosing them in a fieldset.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Needs review
dries’s picture

dries commented

Looks good to me. Make isset() should be is_array()?

budda’s picture

budda
Primary language English
Location Cheshire
commented
StatusFileSize
new removefieldset-is_array.patch528 bytes

isset() vs. is_array() is inherited from the original code. But is_array() makes more sense, patch attached to rectify this.

nickl’s picture

nickl commented
StatusFileSize
new removefieldset_0.patch1.27 KB

The attached patch assumes to fix the problem:

indeed ... IMO, we should only use fieldset when there are 2 or more fields to be grouped

but instead of avoiding the fieldset succeeds in removing the whole forums select box from the form.
Attached a new patch that checks if the form has more than one item and if not will display the 1 item without a fieldset.

nickl’s picture

nickl commented
StatusFileSize
new removefieldset_1.patch1.27 KB

Lets try this again:
The attached patch assumes to fix the problem:

indeed ... IMO, we should only use fieldset when there are 2 or more fields to be grouped

...but instead of avoiding the fieldset it succeeds in removing the whole forums select box from the form.
Attached is a new patch that checks if the form has more than one item and if not will display the 1 item without a fieldset.

nickl’s picture

nickl commented
StatusFileSize
new removefieldset.png11.04 KB

Here is the before and after screenshots.

flk’s picture

flk commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new removefieldset_1_0.patch1.28 KB

corrected syntax @ line 24.

works like a charm in resolving the issue.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Needs work

patching file modules/taxonomy/taxonomy.module
Hunk #1 FAILED at 668.
1 out of 1 hunk FAILED -- saving rejects to file modules/taxonomy/taxonomy.module.rej

nickl’s picture

nickl commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new removefieldset_2.patch1.28 KB

Rerolled to head. Marked RTBC again as nothing has changed.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

drumm’s picture

drumm
he/him
Location NY, US
commented
Assigned: Unassigned » drumm
Priority: Normal » Critical
Status: Fixed » Needs review
StatusFileSize
new taxonomy.module.diff.txt845 bytes

This kinda broke free tags. The #tree is important. Here is a fix which modifies the array a bit less.

beginner’s picture

beginner commented
Status: Needs review » Reviewed & tested by the community

patch #13 also fixes: http://drupal.org/node/83946

AjK’s picture

AjK commented

Pathch #13 :-

Looks good, see also http://drupal.org/node/84108

nickl’s picture

nickl commented

Hopefully this fixes the blockquote issue too.

Patched to fresh head. All seems to be working.

nickl’s picture

nickl commented

Hopefully this fixes the blockquote issue.

nickl’s picture

nickl commented

Patch at #13 RTBC

drewish’s picture

drewish commented

+1 on the patch in comment #13

AjK’s picture

AjK commented

This patch really needs to get committed

Another duplicate http://drupal.org/node/84987

regards,
--AjK

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)