Is there a category for 'nitpicky'? :) Some superfluous stuff may appear when modules such as blog, taxonomy or book aren't enabled. This patch just inserts the necessary checks and moves the $form items into them. Same issue in D6 version but may not be important enough to fix (but do let me know if you want a patch for that, too).

(I'm new to patches so let me know if it isn't exactly right.)

CommentFileSizeAuthor
site_map-admin_settings_form.patch6.97 KBEllen Dee
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

frjo’s picture

frjo CreditAttribution: frjo commented

I'm uncertain what is the best here.

Hiding them as you suggest is cleaner but showing them makes it easier to see what features the module can offer.

Does someone else have some comments or suggestions about this?

Ellen Dee’s picture

Ellen Dee CreditAttribution: Ellen Dee commented

I think it's always confusing to be offered features when they aren't really available, especially to new users. I concur with what you're saying, though. How about instead disabling (rather than hiding) elements and providing messages to indicate that more features are available if x modules are enabled?

Here's an idea:

Break out the module-dependent stuff into their own fieldsets under SITE MAP CONTENT. While we're at it, bring the CATEGORIES SETTINGS stuff into the SITE MAP CONTENT fieldset, since it affects how the category content is generated. (Not sure about RSS SETTINGS, since I've never used it and I'm not sure what it does.)

So you would end up with:

- SITE MAP CONTENT (main fieldset)

  [ ] Show front page

    - MENUS (sub-fieldset)

      .. menu stuff

    - BLOG (sub-fieldset)

      [ ] Show active blog authors <-- if blog not installed, checkbox disabled + (Requires blog module.)

    - BOOK (sub-fieldset)

      --------
      | Books to include in the site map
      |
      | - book 1
      | - book 2    
      --------
        OR
      --------
      | By enabling the book module, you can... bleah bleah (sell it, baby)
      --------
  
    - CATEGORIES (sub-fieldset)

      --------
      | Categories to include in the site map 
      |
      | - category 1
      | - category 2    
      |
      | Category settings
      | 
      | -- all of the settings stuff here
      |
      --------
        OR
      --------
      | By enabling the taxonomy module, you can... bleah bleah
      --------

Thoughts?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Active » Needs review

I think it's better to show this options as disabled and use D7 states to show advanced options when user set checkbox (fieldset seems bad idea)

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs review » Needs work

Agree with Lara!

After looking into code I think that better to reorganize settings form - make menu tabs: General, Content, Taxonomy, Book

Once a module enabled the menu is rebuilded and user could change settings for sitemap but while book or blog disabled the settings just stores a useless variables in memory and confusing user with a huge settings form without any reason.

to see what features the module can offer

Suppose users read a module page before module download

PS: signature should be changed according http://drupal.org/node/224333#hook_forms_signature

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

Filed different issue about function signature #1107786: Fix admin settings form definition

darrell_ulm’s picture

darrell_ulm CreditAttribution: darrell_ulm commented

Looking at this, new feature set should be able to give this option and some other types of flexibility.

darrell_ulm’s picture

darrell_ulm CreditAttribution: darrell_ulm commented

Hi, quick test of the patch above on the 7.x dev branch does not apply at this point, but as per the discussion not sure this is going to happen the way the patch proposed anyway.

Just an FYI

--------------------------
|--- site_map.admin.inc	2010-08-17 12:08:40.768450500 -0400
|+++ site_map.adminNEW.inc	2010-08-17 12:12:21.047450500 -0400
--------------------------
File to patch: site_map.admin.inc
patching file site_map.admin.inc
Hunk #1 FAILED at 39.
Hunk #2 FAILED at 81.
2 out of 2 hunks FAILED -- saving rejects to file site_map.admin.inc.rej

Thank you

daffodilsoftware’s picture

daffodilsoftware CreditAttribution: daffodilsoftware commented
Status: Needs work » Closed (fixed)

@darrellulm I could see that these checks are already committed to the dev branch.So I am making it closed