[UX] move text filter configuration into modals

[klonos]

(Related: #975)

https://www.drupal.org/project/drupal/issues/1834682

Before:

After:

---

PR by @docwilmot: backdrop/backdrop#1023
PR by @docwilmot: backdrop/backdrop#3074
PR by @docwilmot: backdrop/backdrop#3077
PR by @docwilmot: backdrop/backdrop#3096 using dialogs
PR by @docwilmot and @quicksketch: backdrop/backdrop#3131

[quicksketch]

@klonos, I'd be interested to hear your thoughts on https://www.drupal.org/node/1834682, where I proposed moving the filter enabling/disabling to be within the vertical tabs. That issue was held up on debate, but I still think it's worth pursuing.

[docwilmot]

👍

[klonos]

@quicksketch I was just about to file an issue about the very same thing just minutes before you posted your comment above. I was thinking to merge the enabling/disabling with the vtabs and -if possible- allowing tabs to be dragged/reordered. You read my mind. Will share my 2c in that issue + open one for Backdrop.

[klonos]

...in the meantime, this issue here is a valid request I believe.

[klonos]

Sorry guys, but I'm working crazy hours and have only a few hours to spare each day. Anyways, here's what I was thinking...

Perhaps the vertical tabs is not a perfect match for what we are trying to do here. I was thinking that the same model we use in the modules page for the list of modules with the addition of draggability would be mode suitable:

1. It has a checkbox for enabling/disabling.
2. It has a column to hold the filter name.
3. We can have the "more"/"less" link be a "configure" link instead that expands the table row to show/hide the config form of each filter.
4. We can have the configuration summary (currently as a "subtitle" below the tab label) displayed in a second column so that there is a simple overview.
5. We can add a drag cross and enable reordering.

This way, we merge all three sections of that page (enabling/reordering/configuring) into a single one.

What do you guys think?

[quicksketch]

I think this sounds like a pretty good plan of attack. We don't have a real pattern for this UI, but it sounds like it may work well.

[klonos]

Yeah, originally I thought that perhaps we can create some sort of "hybrid" vtabs with a checkbox on their label (to enable/disable filters), but then I figured that the checkbox can reside within the vtab. That was when I was thinking to merge only the enabling/disabling section with the config section. That's when I had a "Wait a minute" moment and said if the checkbox was not on the label, then there's room for a drag cross in there. That would allow to merge everything in one UI.

More thinking and browsing around the admin UI (plus having #1005 and the options element at the back of my head) got me considering the field UI and the modules page too. So the "hybrid" idea seemed to make more and more sense to me, but instead of the vtabs it seemed that expandable (in order to hold the config form) and draggable (in order to provide reordering functionality) table rows
was a more suitable element. Perhaps we can even style them to resemble vtabs if needed.

Another thought on the table is to use modals for config instead of expanding the table rows.

Separate issue then? ...or re-title and re-purpose this one?

[quicksketch]

Separate issue then? ...or re-title and re-purpose this one?

Let's keep using this one. It'd be nice to see the options that we're exploring in one place, especially if this idea doesn't pan out.

[docwilmot]

This seems a really good plan. I'd vote for modals for the config.
How would we handle disabled filters?

1. Keep them in the draggable list in the same position they were before being disabled?
2. Keep them in the list but float them to the bottom and gray them out?
3. Have a separate section below the list for disabled filters?
4. Hide them completely and have a button/link to "Add a filter"?

[klonos]

Hmm, thinking about this and will get back with (more) comments...

In the meantime, I took the liberty to number the options you list in your comment @docwilmot so we can reference them in future comments. Hope you don't mind.

...off the top of my head, I don't particularly like option 4 unless that list gets reeeeally big. I prefer the user to be able to see the available filters without extra clicking around if possible.

[klonos]

...here's a thought:

• We ship with a sensible order for all default filters.
• Enabled ones at the top, disabled ones at the bottom (not 100% sure here because some filters might require specific ordering [UX] Add checks to make sure that users do not come across WTFs while configuring text formats. #1033).
• User can reorder and enable/disable at will.
• We provide a "reset to defaults" link that does the obvious.
• Once user has moved things around, we keep disabled filters in user specified place (user might change their mind or simply disabled for troubleshooting purposes).

Just thinking out loud here.

[docwilmot]

Reset to default option won't work once custom module-provided filters are enabled. There be no real "default" then.

I think number 3 is the Drupal and Backdrop standard. Think the old blocks list, Display Suite, etc.

[docwilmot]

Is this something like what we had in mind (minus the treatment of disabled blocks)?

Option 1

Option2

Option3

[docwilmot]

Other option is not have separate enable and configure links but do the enabling in the configure modal.

[MrHaroldA]

I'd suggest to keep it as close to admin/structure/types/manage/page/display as possible.

[jenlampton]

I disagree with @MrHaroldA since the manage field display page will likely not be around for long (we should be using layouts for that instead). Let's think this through on it's own merits only.

One thing to consider - in none of the options in @docwilmot's comment can you see which HTML tags are allowed without another click. One of the things that bothers me most about the current filter settings is that I need to click (into the vertical tab) to see that list.

There's a lot of space in this list, however. Maybe we can add an additional 'summary' column that can display useful information (like allowed HTML tags)?

[jenlampton]

Another option as proposed in https://www.drupal.org/node/1834682

Also worth a read: https://www.drupal.org/node/1087100

[klonos]

Yes, each time I read through the commends on that issue I feel so frustrated at how things (do not) move in Drupal land. Maybe I am feeling this way because I share @quicksketch's point of view and I saw a great UI improvement idea being shot down so many times over potential (and may I add pre-existing) security issues that by the way we can solve here by implementing #1033.

I really do not care about the actual implementation (be it vertical tabs or dragable table rows), so long as we solve the main issue here of consolidating the filter enabling section and the filter settings section. That way users would see that there is something to configure (settings section) once they enable something in the enable/disable section. Consolidating the filter ordering section as well would be an added bonus.

So far, I like @docwilmot's option 3 (minus the spacing and alignment details). So, the order of elements I think should be: drag cross, enable/disable checkbox, filter name, configure link. I also like the idea of the configuration living inside modal dialogs.

@jenlampton: either way (dragable list or vertical tabs), you need an additional click to see the list of allowed HTML tags. Unless I'm missing something. We can have those shown in a "more" link that expands the row (à la modules page "required by" section). Still would require an additional click though :/

[docwilmot]

in none of the options in @docwilmot's comment can you see which HTML tags are allowed without another click.

My mockup was incomplete, just to demo the order of things. The individual draggable rows could have information about the filters below the filter name.

And I don't really like the vertical tabs for filters to be honest; it doesnt seem like this is an appropriate use of vtabs.

I'd suggest to keep it as close to admin/structure/types/manage/page/display as possible.

I suspect @MrHaroldA was referring to copying the UI mechanism of draggable rows above with disabled rows below.

[klonos]

The individual draggable rows could have information about the filters below the filter name.

👍

[docwilmot]

I was thinking of starting to try to do this, but thought I'd get consensus first:

tabs or draggable rows?

My first 13 votes are for draggable rows.

[quicksketch]

The individual draggable rows could have information about the filters below the filter name.

I think this is a good idea, especially if we can include summary-like information, such as the current allowed list of tags.

I think nixing vertical tabs is also a good idea. Most of the time, settings on filters does not need to be changed, especially now that CKEditor will automatically adjust the tags based on the toolbar configuration. This will greatly simplify the overall configuration page for filters.

If we go with dialogs here, we'll probably use a new mechanism that we haven't used in the past. Right now our other dialogs (Layouts and Views) both use AJAX-saving to temporarily save configuration until the entire page is saved. I think that may be overkill here, and we'd need to completely change the way filter settings are saved to use that approach. Instead, what we'll probably want to do is leverage some of the more typical jQuery UI functionality, where we simply have a dialog open and display part of the existing page, without an AJAX request. The non-JS fallback would be identical to the page as it is today, with just all the settings for all filters being shown on the page (the configure links should also not be shown in that case).

We haven't done this particular kind of implementation before in Backdrop core. It will probably require a small amount of JS in filter.admin.js to wire it all up to the existing dialog system.

[docwilmot]

First pass, minus @quicksketch's good advice re the filter saving, which is of course the right idea, but I hadnt seen it. But at least we can evaluate this new UI.
See https://github.com/backdrop/backdrop/compare/1.x...docwilmot:filters?expand=1
Saving DOES NOT WORK. This is just a UI test.

[klonos]

...you know me, I'd love to check it out, but I still haven't sorted my local environment issues (#1072).

[klonos]

...alright. Back in business!

1. A couple stray dpms here and there (I don't have devel.module installed) - once lines were removed, I could test:

   Fatal error: Call to undefined function dpm() in C:\www\backdrop-issue1032-comment-127066130\core\modules\filter\filter.admin.inc on line 205```
   

   Fatal error: Call to undefined function dpm() in C:\www\backdrop-issue1032-comment-127066130\core\modules\filter\filter.theme.inc on line 105
   

2. Checking/unchecking the enable/disable checkbox would move the respective filter to/from the disabled section. "Manually" dragging and dropping would not check/uncheck the respective checkbox though:

   

3. Saving the form takes you back to the filter list page with both an error and a success message. The error is:

   Notice: Undefined index: order in filter_format_save() (line 221 of C:\www\backdrop-issue1032-comment-127066130\core\modules\filter\filter.module).

4. Re-editing the filter shows that all filters are moved to the disabled section with their checkboxes unchecked.

   All in all and glitches aside (I realize this is a WIP and you did say "Saving DOES NOT WORK.") UX-wise this is looking really really great @docwilmot. Thanx!!!

   ...just a few more minor things:

5. I think that the enable/disable checkbox column should come second, after the drag cross. That is if others agree too. I mean, that way it will be closer to where we have the checkboxes in other pages like the modules page for example.

6. The column header titles do not make great sense as they are. Perhaps "Filter weight" should be "Filter order" or simply "Filter" and "Configure" should be "Operations" so it can accommodate any future actions that we may implement and in order to be in tandem with other places where we use tables.

[docwilmot]

Thanks @klonos. Please note the statement above:

Saving DOES NOT WORK. This is just a UI test.

This isnt near finished yet, I know, sorry; the drag/drop worked fine earlier, I must have changed something and not rechecked. Please note my jQuery skills are laughable.

Excuse the "Empty"; just temporary; same for the dpm()s

How does it work as an alternative UI for filter formats though? Is it intuitive? Friendly? Friendlier than previous?

[klonos]

I know, notice my statement:

All in all and glitches aside (I realize this is a WIP and you did say "Saving DOES NOT WORK.") UX-wise this is looking really really great @docwilmot. Thanx!!!

So, yes very user-friendly and a great improvement over what was there previously. Thanx once again!

[docwilmot]

Ahh we cross posted there.

Thanks for the review; I tried the checkboxes second but note that this also goes contrary to common standard which is the cross-hairs should be associated closely with what youre dragging; we're dragging the filters, not the enable checkbox.

That, and the fact that it doesn't look at all right to me. Perhaps if having the checkbox there is unacceptable, we could opt for enabled/disabled links instead?

Agreed with the "Filter weight" header being wrong. Not sure about operations though; right now, all you can do with filters is configure them. There are no other operations.

[klonos]

...just a thought here. In order to simplify code and UI... do we actually need both ways of enabling/disabling? Should we just keep the checkboxes and ditch dragging to a "Disabled" section? In that case, should we just do what we do for Views UI (gray-out and disabled rows)?

[quicksketch]

I'm going to work on addressing the feedback from the latest code review.

[quicksketch]

New pull request up at backdrop/backdrop#3131

I reworked the updating of the hidden "filtered_html" element and fixed the interaction between the configuration of CKEditor's buttons and the tag list. It should now work consistently when you drag in and out buttons like Image or Strike-Through.

I also encountered a number of problems when creating a new text format, some PHP errors would be thrown when configuring filters before giving it a name.

We need a code review and some additional testing on this.

[jenlampton]

I'm testing the latest PR, thoughts:

• In my testing all the tags I needed based on my editor configuration were added to the Limit allowed HTML tags filter configuration.
• I really like the new text that explains why tags have been added:
  Based on the text editor configuration, these tags have automatically been added:...
• I love the new disabled color change for formats not currently in use.

These should maybe be a follow-ups:

• when creating a new text format I struggled to determine how to turn on a filter, since the "Enabled" column wasn't on the left hand side of the filter name.
• In the new modal, I think the Submit button should have a value other than Submit but I realize we may need to adjust the jQuery if we change it.

[quicksketch]

I also thought that the checkbox might be more expected in the left-hand column, similar to enabling modules or any of the views bulk operations forms (i.e. admin/content).

We can change the dialog submit button very easily, it's just the label on the button, no need to adjust any JS.

[jenlampton]

We can change the dialog submit button very easily, it's just the label on the button, no need to adjust any JS.

Ah! Maybe it's the tests that will need to be updated?

[jenlampton]

I added a few commits to the PR to update the CSS (adding RTL styles for filter UI) and updated the docs in one place to indicate Filters are stored as config, not in the database anymore, but as long as we can get someone else's eyes on that before it's merged, this issue is RTBC!

[quicksketch]

Changed the dialog button label from the generic "Submit" to the slightly more specific "Update".

[quicksketch]

when creating a new text format I struggled to determine how to turn on a filter, since the "Enabled" column wasn't on the left hand side of the filter name.

Let's consider moving the checkbox for enabling as part of the follow-up we already have to style the Configure buttons over at #4378.

[jenlampton]

Let's consider moving the checkbox for enabling as part of the follow-up we already have to style the Configure buttons over at #4378.

Great, adding it there now :)

[quicksketch]

Merged backdrop/backdrop#3131 into 1.x for 1.16.0.

Thanks everyone for the extensive feedback and testing!

HUGE THANK YOU to @docwilmot for spearheading this and making not one but TWO entire implementations to get this done. Really happy with how this has turned out.