'Unpublished' title and desc entry fields on admin form
| Project: | Web Links |
| Version: | 6.x-2.0 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
Hi Nancy and Robert,
In the function _weblinks_unpublished() in weblinks.module the name and description are retrieved from weblinks variables, as follows:
<?php
$noclass->tid = 'unpublished';
$noclass->name = variable_get('weblinks_unpublished_title', t('Unpublished'));
$noclass->description = variable_get('weblinks_unpublished_desc', t('These links are not published and need to be reviewed.'));
?>However, I would like to change this text so went looking for the entries in the settings pages - but could not find them! Maybe they were never added or got dropped accidentally during a re-design. I have added them to the General tab, in the Group Settings fieldset, beneath the similar fields for the Unclassified group. See attached image.
I noticed that the entry field for the Unclassified description was hidden by javascript when the title was blanked, so I have repeated this for Unpublished. Actually, I have improved it because often when entering text you tab forward and backwards, but the javascript was only reacting to a click in the title field, so I have added 'focus' and 'blur' to the events that will trigger the show/hide.
Then during testing I found that if the title was blanked (as is mentioned in the help text) and you also have collapsed as the default for that group then you cannot open the fieldset because there is no title to click on! I would like to suggest that to we cross-validate the entry values, for example, if the title has just been blanked then the validation will fail if that group is set to collapsed. Likewise, on the group settings tab, if the unclassified or unpublished group has been set to 'collapse by default' then the validate will fail if the group has no title, or the checkbox could be greyed to stop it being changed. The value of 'collapse all groups' may also need to be considered in the cross-validating, as this overrides the individual group settings.
If you think this is overkill and the admin should just see the fault and fix it manually then that's ok, but I am happy to do the coding and let you take a look. Or I could just post the patch for what I have done so far.
Jonathan
| Attachment | Size |
|---|---|
| weblinks_group_settings.jpg | 99.12 KB |
#1
I all likelihood what I did was to duplicate the code from the unclassified handling. Personally, I don't see a need to have a setting for this at all. We should just hard-code "Unpublished" and then none of this matters.
Robert?
#2
I don't like the idea of not being able to change the word 'Unpublished' - so many other things are customisable in weblinks that this would seem unnecessarily restrictive. There are several reasons why a link might be in the unpublished group, eg it could have been unpublished by the checker, it could have been added by a user who has to get approval from a moderator before publishing, it could be set for future publication by the scheduler module, etc. Also it is quite reasonable for the group title to be blank, as depending on your theme you may not want the text (and it seems unreasonable to force the admin to modify the css or .tpl just to hide the title).
But OK, if you really want the fixed word 'Unpublished' then we must at least have the ability to enter a description for this group, which is omitted at present.
Regarding allowing a blank title, I had another idea. Instead of making checks in the validation routine, it could be done at page rendering time, so that if a group was going to be collapsed but there is no title then it is simply not collapsed, overriding the admin settings. Not saying this is better, just another alternative to think about.
Jonathan
#3
Actually, I've had another thought about blank titles. I think it would be better to always force some text in the group title for Unclassified and Unpublished, just like the admin-defined groups do. Currently if the title for Unpublished or Unclassified is blanked then in the group settings tab the row just has 'Show' with no way to identify what group it is. If the normal groups have to have a title then so should the Unclassified and Unpublished, and this would be a simple way to resolve the 'cant get to a collapsed group because there is no title displayed' problem that I found yesterday.
If we wanted to allow the group titles to be non-displayed for Unclassified and Unpublished, then we should do it for all the groups and introduce a 'do not display the group title' option in the Group Settings tab. But this is not a requirement for my sites, so I am not proposing to write this now. I'd just like to add the missing entry fields for Unpublished title and Unpublished description and validate that the two titles cannot be blank.
Jonathan
#4
You bring up a point that has many facets. "Unclassified" and "Unpublished" should also be block names (don't have time to check now), so yes, something needs to be there. I am less concerned about the configuration of "unpublished" than I am "unclassified" because there are people who don't group their links at all. But unpublished is a special situation that should be mostly visible to an admin or sub-admin, so they should know what they are looking at.
#5
How about this: if the "unclassified" or "unpublished" title is blank, the group doesn't display at all?Bad idea...On my system, if the title is blank, the field set is not collapsible, but still shows up.
I'm going to make the unpublished title required.
#6
Committed to -dev. If the blank name is still a problem on your system, please open a new issue.
#7
I coded and tested a change for this yesterday and was going to post a patch for you today!
I forced the titles for both to be mandatory so that the blocks and groups always have an identifiable name, but I added the option for any group not to display the text (to cater for your example of sites which do not categorise). I also validated that you cant both collapse and not display the title, and resolved this conflict at render-time too. It seems to work well, so I will take a look at your solution and see whether it is worth expanding it for the extra flexibility I added.
Thanks for doing this, and also for all the work you do on weblinks. It's a great module.
Jonathan
#8
Which Drupal release are you using? Theme?
I have 6.16 and Pixture-reloaded and Garland. The fieldset with no name is simply not collapsible, which I think is fine for the unclassified. I had to make unpublished required so that users would know the difference between the two groups when displayed. For those who don't classify links, a blank name makes the most sense.
#9
You say 6.16 but Drupal is only on 6.12, which is what I am on. I am using weblinks 2.0 and a Zen-based theme. Are you saying that even if you select to collapse a group with no title, then the theme overrides this? I've just tried it with Garland and the group is collapsed, just as before, with no title to click to open it.
Yes, I absolutely agree that for users who dont classify their links then they probably dont want the title. And not to collapse with no title is correct. But to blank out the title means that weblinks blocks are not so easily identified. So I forced both the unclassified and unpublished titles to be entered but added the 'do not show title' option for each group, provided that they are not collapsed.
I'll re-work to incorporate your latest dev, then make a patch for you to see what I did.
#10
Ah, the occupational hazards of having to maintain multiple versions... Yes, 6.12.
With the -dev version, with the group settings moved to the group add/edit screen, the only way to mark the unclassified and unpublished groups as collapsed (well, unless you get creative and go into the variables table) is to mark all groups as collapsible. Yes, that needs to be worked on, because doing that, I do see the problem you mention.
#11
In
function _weblinks_format_group, I made a simple change:$fieldset = array('#title' => $term->title,
'#collapsible' => $term->collapsible,
'#collapsed' => $term->collapsed,
'#value' => $data,
);
to:
$fieldset = array('#title' => $term->title,
'#collapsible' => $term->collapsible,
'#collapsed' => $term->collapsed && !empty($term->title), // Can't be collapsed if the title is empty.
'#value' => $data,
);
I also added a note on the settings page about this.
Committed to -dev
#12
If you're happy with the change, please mark this issue "fixed."
#13
Yes, Nice fix.
I tested the new code, and had one observation: When setting #collapsed, I would also set #collapsible to have && !empty($term->title). Currently the class 'collapsible' is still added to the weblink when the title is blank, even though it is not now collapsible. For consistency with themes which may style non-collapsible groups differently, I suggest adding this so that neither collapsed nor collapsible are set when the title is empty.
I tested this yesterday, but have since reverted my local site to a pre- non-overview version which I need to get in to production, so I can't check right now. But I think that was the only suggestion I had.
Jonathan
#14
change committed to -dev
#15