Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is a mockup for the Filter UI. Currently it is soo complex, that I dare to call it broken.
Please comment on this, for I will not ake any patches, when the general idea is disliked.
The idea is t split filters and input formats better. Filters are filters, defined in modules. Input formats are bundles of filters. This is how we have it ATM, but the interface fails to communicate that. I tried to develop a consistent (with the rest of Drupal) interfce that makes it clearer what is what.
The menu is as follows:
* administer
** ....
** settings
*** input formats
*** filters
** ...
Comment | File | Size | Author |
---|---|---|---|
#55 | filter.module_13.patch | 20.33 KB | m3avrck |
#54 | filter.module_12.patch | 20.31 KB | m3avrck |
#52 | filter.module_11.patch | 28.03 KB | m3avrck |
#46 | filter_interface_improvements_3.patch | 21.49 KB | Bèr Kessels |
#45 | filter.module_10.patch | 20.27 KB | m3avrck |
Comments
Comment #1
Bèr Kessels CreditAttribution: Bèr Kessels commented* administer
** ....
** settings
*** input formats >> see attachement 1
*** filters
**
Comment #2
Bèr Kessels CreditAttribution: Bèr Kessels commented* administer
** ....
** settings
*** input formats >> see attachement 2, tab 2
*** filters
**
Note: The default checkbox lmay seem a bit odd. But checking it will remove the "default status" from the current "default" one and make this one "default". The help text, and a drupal_set_message should explain this.
Comment #3
Bèr Kessels CreditAttribution: Bèr Kessels commented* administer
** ....
** settings
*** input formats >> clicking a 'configure' link in the table of attachement 1
*** filters
**
Comment #4
Bèr Kessels CreditAttribution: Bèr Kessels commented* administer
** ....
** settings
*** input formats
*** filters >> see attachement 4
**
Comment #5
Bèr Kessels CreditAttribution: Bèr Kessels commented* administer
** ....
** settings
*** input formats
*** filters >> clicking a 'configure' link in the table of attachement 4
**
Comment #6
Bèr Kessels CreditAttribution: Bèr Kessels commentedIMO this makes the interface easier. But other disagree. And because it also removes one feature: (being able to use E.g. HTML in different input formats, with different settings)
I'll simply wont fix this :(
Comment #7
Bèr Kessels CreditAttribution: Bèr Kessels commentedI decided to go for it anyway. Be it in a slightly different way then my initial mockups. The difference is that I left the filters where they are, hidden beneath the formats.
So, here is the patch that makes the filter UI more consistent with screens like blocs, menus, vocabularies et al
also nice to note is that:
93 lines are added, 104 are removed. So netto we have less code :)
(cvs diff filter.module | grep ^+ | wc -l)
Comment #8
Bèr Kessels CreditAttribution: Bèr Kessels commentedHere is how the listing now looks.
Comment #9
Bèr Kessels CreditAttribution: Bèr Kessels commentedAnd the "configure" screen.
The 'add' screen looks exactly the same, only with the default values pre-filled.
Comment #10
Bèr Kessels CreditAttribution: Bèr Kessels commentedOh, And here is a really nice example of why the current screen is no good.
And here I "only" have four roles. I run a site with 12 roles, imagine this screen then!
Comment #11
Bèr Kessels CreditAttribution: Bèr Kessels commentedAnd here is a pathc for HEAD (I forgot that I developed this on a 4.6.2 codebase. Sorry)
Comment #12
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedI like this very, very much!
This is - indeed - much better than the current UI. This always fits, and makes the life of people who use a fixed width theme much nicer..
Ber, FYI (you probably know this too), 'admin/access' does also break fixed width themes due to the fact that it expands the width of the table when more roles are being added..
Good catch! I like it much more this way..
+1 from me...
Comment #13
Dries CreditAttribution: Dries commentedThe code style needs work (spaces, tabs, 'as' -> 'AS'), the code comments need work, and debug statements should be removed. Screenshots look good to me.
db_query('UPDATE {filter_formats} SET cache = %d, name = \'%s\', roles = \'%s\' WHERE format = %d', (int)$cache, $name, $roles, $format);
can be written better/shorter quote-wise.Comment #14
Bèr Kessels CreditAttribution: Bèr Kessels commentedDries, thanks a lot for the review. OI fixed your comments. Or so I hope. code-style.pl was of no use (can that *please* be removed from core?) because it chokes on the help texts. It also pointed out a lot of old formatting isues, whic, when fixed wuold flood this patch with unrelated fixes. I tried to narow my style-searching to the areas I touched. And I made the comments more verbous.
The query is fixed. it saves no characters, but looks better now. Thanks for the tip.
The print_r..... blush. thats what I get for not running my default grep -r print_r on my code after finishing up...
I could not find a lot of spaces and tabs. So it lmight be that i missed one. I triple checked it though.
Comment #15
Steven CreditAttribution: Steven commentedMoving the role settings inward does fix the wide-table problem, but it makes the filter configuration much more opaque. We could perhaps fix this by showing the roles as a comma-separate list in the input formats table. Such a list can wrap and will not stretch the table.
Other than that:
Comment #16
Bèr Kessels CreditAttribution: Bèr Kessels commentedconsistancy, consistancy and more consistancy.
There really is nothing worse that having a different 'concept' of UI for every thing in Drupal. We are going in the right direction, with the blocks being editable objects, menus acting like that, taxonomy, users, etc..
I wanted to make this act similar. I am not at all happy with the fact tah the list is still a form. But we need this, for setting the default.
A big -1 for a comma-separate list. That is extremely hackish, even worse useabilitywise and just silly
* we should avoid typing when not needed.
* we should avoid errors on input, when they can be avoided (a typo is made very easy, I know all about it) Form set errors won't fix that usability -wise.
I agree that you sort-of loose the overview. But IMO that is a minor loss comared to what we gain, usability-wise.
The wtoo-wide was onlywhat triggered me. But do not think that I am noly making this patch to fix that issue. This patch is there to improve usability, through consistancy. o make drupal behave the same in most places in admin. If you know one screen, you know most of them. We really should move away from custom-hacks for every page, because that is useability rule #1: consistancy. Stadardisation, even if sometimes a custom hack might make things easier in that single case, it will reduce your overall usability so much that nearly ever it is worth that hack.
That select-boxes instead of the table issue: consistancy. As well that it completely broke fluid CSS layouts, my solution is more consistant, and uses forms the way they are meant. If we cannot use $descritption in checkboxes they should be removed. but IMO it feels much more consistant. Tables are for tabular data, and this is simply a list of items, with additional data.
And i do not really get what you mean with "too far down". Do you mean that you would like to see the order of the groups different?
Comment #17
Dries CreditAttribution: Dries commentedI haven't tested Ber's patch yet but looking at the screenshots, it like what I see. I remember being confused by the current filter UI, and often, I still am.
Comment #18
moshe weitzman CreditAttribution: moshe weitzman commentedI dislike the recent movement away from overview pages. I think we can still provide and overview while allowing editing to happen in a dedicated form. An example in the wrong direction is 'content types' admin. It is impossible to get an overview, and the listing page is worthless. I agree with Steven on this one.
It is true that consistency is desirable. But consistent crap is not.
Comment #19
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI agree with Moshe. Getting a quick overview about what is set how is essential for an admin.
Comment #20
Robrecht Jacques CreditAttribution: Robrecht Jacques commentedOverall I like the screenshots. Did not test the patch.
Bèr, what I think Steven (#15) meant by "showing the roles as a comma-separate list in the input formats table" is that as an overview page it would be better if you added a list (not an input box!) to screenshot "input-formats_list1.png" (comment #1). Add a column that reads "Roles" (or "Enabled for") and then for each row a comma seperated list of the roles that can use this input format (or maybe even better a HTML list). You would still need to click "configure" to _change_ the roles.
The "Default" text (not a radio button) you already show in the "Options" column is something similar: it immediately shows that this input format is the default without you having to go to the "configure" page. Why not do the same for "roles"?
If this is what Steven meant, then I tend to agree with him.
And to help moshe and killes: maybe you could even add a column that _lists_ the used input filters. Again, only a list, not a way to change it. That's what the "operations" column is for.
A table like that will "fluid" better than what we have now, still it is a _complete_ overview of the settings.
I could have misunderstand someone... I have the tendency to do that...
Comment #21
Bèr Kessels CreditAttribution: Bèr Kessels commentedComment #22
Bèr Kessels CreditAttribution: Bèr Kessels commentedby popular demand: a comma separated list.
Comment #23
Bèr Kessels CreditAttribution: Bèr Kessels commentedand a screenshot
Comment #24
Dries CreditAttribution: Dries commentedPlease check your use of
print theme('page', $output)
. Normally, you just return$output
.Comment #25
Bèr Kessels CreditAttribution: Bèr Kessels commentedAll instances of theme('page',...) in filter.module are now changed to return ... ;
furthermore, the patch is the same.
Comment #26
Uwe Hermann CreditAttribution: Uwe Hermann commentedBèr, I think you forgot to attach your updated patch.
Comment #27
Bèr Kessels CreditAttribution: Bèr Kessels commentedmy email client has this nice warning system for attachements. Maybe drupal should search for the word attachement too, and when no att. found give an error ;). Or maybe i should just learn to pay attention. Guess that is easier.
Anyway, here it is.
Comment #28
Dries CreditAttribution: Dries commentedI wanted to apply this patch but:
1. I can't change the default input format. 'Save'-button doesn't work.
2. I got confused by the fact I can't change the roles of the default filter. I think the form group description should explain this.
3. form_group(t(filter)) should be form_group(t('filter'), ...).
Comment #29
m3avrck CreditAttribution: m3avrck commentedFound a few more issues:
If I go into 'Configure' for 'PHP Code' and click the 'Configure' tab at the top and then click 'list filters' link in the text, I get this warning with PHP5
warning: Missing argument 1 for filter_admin_format() in \drupal\modules\filter.module on line 378.
Also, it is sort of confusing as to what the 'List' tab implies. For example, if I click on 'input formats' in the admin menu, I get a list of the current filters. When I click 'configure' for one of those, I see the options for that filter. However, it says 'list' in one of the tab, which doesn't really mean much. I thought that was the list of filters, not the options for that filter. That should be cleaned up. Also, there is no way to get back to the full list of filters unless you click on 'input formats' in the admin menu. A setting/link to fix this would be great.
Also, where is the link to add filters???
I think the options/layout/tabs should mimic of that of admin > users.
So on admin > input formats, it has the tabs 'list', 'add format' ... very clear and consistent.
On a configure page for a filter, it should have 'list', 'view', 'configure', 'rearrange' ... this would make it easier to navigate and get back to the original list of filters.
Make sense? I think this and Dries' comments put this patch over the top :)
Comment #30
tag-1 CreditAttribution: tag-1 commentedIn my initial battle to figure out this module's (quite nice) functionality, what tripped me up the most was actually the nomenclature.
The way I keep it straight in my head now is to translate "input format" to "filter set". If you view those (the current) screens with this in mind, I think it's a lot easier to understand what's what. It sort of describes this in the help text, but the terms "filter" and "input format" don't really convey the relationship between the two -- there's no way to intuit that an "input format" is a collection of "filters". Not to my mind at least... and well, we know nobody reads instructions...
Are there any technicalities that make renaming "input format" to "filter set" not accurate? Or does someone have a better term/terms to better show the relationship of these elements? (I'm aware of filters in servlet architectures but I don't think there's much of a collision with that usage).
Anyhow, I guess this doesn't quite address the UI controls and/or layout, but would still help a lot.
Comment #31
Bèr Kessels CreditAttribution: Bèr Kessels commentedI prefer filter set. In fact: I like it a lot. Anyone else ideas on this?
Comment #32
Dries CreditAttribution: Dries commentedI too had problems with the terminology; it took me months to get used to 'input formats' versus 'filters'. Using 'filter sets' might improve the situation -- especially from an administrator's point of view when you have to wrap your head around the UI/difference. I'm not native English, but 'filter set' sounds more explanatory, yet slightly more geeky so I'm not sure if it flies for users who don't need to understand the underlying concepts (eg. under the node and comment submission forms).
Comment #33
Bèr Kessels CreditAttribution: Bèr Kessels commenteda heads up: I ma redoing the patch. But decided no to change the name yet. That is a too big change. IT should be in a separate issue, IMO.
Comment #34
Bèr Kessels CreditAttribution: Bèr Kessels commented* Bugs as reported by dries fixed.
* The [add] tab not appearing is due to your menu caching, refresh that please.
* Fixed an additional bug: Some agents do not send TRUE for disabled checkboxen. Also in current filter admin.
and people: we have a problem. Deleting a filter trows errors, due to the revisions changes. that happens in filter module now, but also after this patch. I have too little knowledge of the filter system to fix that, and IMO that is a separate issue. It should be fixed right after this patch is committed.
Comment #35
Bèr Kessels CreditAttribution: Bèr Kessels commentedFYI: the delete bug is waiting here: http://drupal.org/node/30781
Comment #36
m3avrck CreditAttribution: m3avrck commentedGet this warning in PHP5:
Warning: Missing argument 1 for filter_admin_format() in \modules\filter.module on line 379
I have traced this error back to line 240. Why are there no callback arguments like above on line 235? Looks like a source of a problem/bug here so needs to be addressed, not sure exactly how to fix it myself (otherwise I would). Should be easy it seems.
Also, for a quick and easy usability improvement, on line 239, change
t('list')
tot('view')
... makes the menus less confusing.After those fixes, looks like this patch is ready to be committed :)
Comment #37
m3avrck CreditAttribution: m3avrck commentedAlso, this dialog is a bit confusing:
Maybe make it so that first line doesn't appear there on that page?
Comment #38
Bèr Kessels CreditAttribution: Bèr Kessels commentedComment #39
Bèr Kessels CreditAttribution: Bèr Kessels commentedchanged that line of help.
Can this please still be taken in consideration before the freeze?
Comment #40
m3avrck CreditAttribution: m3avrck commentedCan't apply patch, getting error:
Patch malformed at line 287
Comment #41
Bèr Kessels CreditAttribution: Bèr Kessels commentedstrange. the dowloaded one breaks too, yet the one i uploaded stll works.
anyway, rerolled
Comment #42
Dries CreditAttribution: Dries commentedIMO, the form function needs more work. You need to group the form construction and the form saving. Right now, when you don't provide a title, you are given an error and an emptied form. This may be a left-over from the original code though but it would be cool if you could tidy this up.
The form_group() description still need a trailing point.
There is a broken link in the following help text on the 'add input format'-page: If you notice some filters are causing conflicts in the output, you can [rearrange] them..
Comment #43
Bèr Kessels CreditAttribution: Bèr Kessels commenteddont have time anymore. sorry.
Comment #44
m3avrck CreditAttribution: m3avrck commentednew patch soon!
Comment #45
m3avrck CreditAttribution: m3avrck commentedOk here's a new patch. Lot's of fixes in this one (all of the ones from above except the one noted below), including support for the new revisions patch.
Additionally, 'input formats' has been renamed to 'input filters'. After debate on IRC, and the above comments in this post, it seems that 'input formats' is somewhat misleading and doesn't make sense. 'input filters' clears this up and makes sense to both native and non-native English speakers (as tested in IRC anywho ;)). This also makes more sense since Drupal refers to everything as 'filters' and not 'formats'.
However, there is one issue still left unresolved: If you create a new input filter, and don't specify a title, page reloads clearing all filled in fields and prompts the error message. Additionally, a blank filter is created.
Wanted to get this patch posted, I'll look at it later today but if someone can get this fixed easily, please do, thanks!
Comment #46
Bèr Kessels CreditAttribution: Bèr Kessels commentedthis patch should fix the issue m3 rasies abuot amepty formats being created.
Yes, i use a simple drupal_goto, in case of a for error. And no, it is not very easy to get the old posted data back.
Comment #47
moshe weitzman CreditAttribution: moshe weitzman commentedthe most visible place for this 'input format' term is on the node form. thats where ordinary users see this stuff. in that context, i don't think 'input filters is an improvement over input formats. the question we are asking our users is 'what is the format of your post'?, not what filters do you want applied ... just my .02. sorry i wasn't in IRC when this came up. this need not hold up the whole patch.
Comment #48
Bèr Kessels CreditAttribution: Bèr Kessels commentedMoshe. I agreed with you, and was against the name change. In this patch at least.
However, it slipped in.
Stetting code to be committed for it has gone trough so many review cycles that this patch has MUCH better code than the rest of the filter module, which REALLY needs a big overhaul :)
Comment #49
Dries CreditAttribution: Dries commentedAre you saving the data before validating it? If we want to convert this form to the new form API, we have to fix this cleanly. Sorry.
I agree with Moshe that 'input filters' is suboptimal. People want to select how their text is going to be processed. The wordfilter sounds rather technical to me. Also, the difference between a single filter and a set of filters becomes rather blurry to the point it could be very confusing.
Comment #50
m3avrck CreditAttribution: m3avrck commentedWell I was the one that changed 'input filters' to 'input formats' as that seemed to be the best thing we could come up. I guess it got confusing because people wanted this to change but maybe not in this patch. So I will revert that change in a forthecoming patch.
Additinally, this patch is *not* ready. I was going through the form logic last night and it is a bit off, hence why errors are displayed along side a an empty form.
I'm going to work as hard as I can and as fast to clean this patch up like I mentioned I would yesterday. I'll have some ready soon enough that will be commit worthy.
Comment #51
Bèr Kessels CreditAttribution: Bèr Kessels commentedaccording to me its a go or leave. if m3 or anyone else wants to take over fine. I am out. no time.
Comment #52
m3avrck CreditAttribution: m3avrck commentedOk new patch! Everything is fixed. Code has been reworked and lots of bugs fixed. Code should also be simpler to read (or at least follow in terms of logic now). Ready to go!
Comment #53
Souvent22 CreditAttribution: Souvent22 commented+1. I honestly have not played around with the filter formats ever. I looked at them, adn they were a mess. Did the patch, and everything was clear as day. Anything that makes drupal easier to use...AND makes it easier to understand and make sense should def. be included.
Comment #54
m3avrck CreditAttribution: m3avrck commentedUpdated patch with Dries fixes in.
Comment #55
m3avrck CreditAttribution: m3avrck commentedOne more cleanup, fixed a 'hairy' issues. Code is solid, implementation 110% (to the best of my abilities over the past 2 days, devoted way too much time to this patch I think ah well). Ready to go!
Comment #56
Dries CreditAttribution: Dries commentedCommitted to HEAD. Thanks. It can be "form API"-ied now.
Comment #57
(not verified) CreditAttribution: commented