revolution
revolution copied to clipboard
modxcms
[3.x] Refine Quick Create/Edit Windows for Elements
What does it do?
Made changes to improve the layout of the quick create/edit TV window:
- Moved select TV entries to the default topic to make them available application-wide
- Adds in the help descriptions to most fields
- Updated SCSS to match field styles more closely with the main editing panel
- Changed the window mode to modal and darkened the overlay to better separate the window from the main view
Below is a screenshot of how the window should appear:
Why is it needed?
- As it stands, the quick create/edit (QCE) forms get visually lost against the main MODx interface below. Changing the window to a modal allows better focus on the task at hand.
- Makes the QCE format more consistent with the associated main editing panels.
How to Test
- From your terminal app, run
grunt buildfrom within the_build/templates/defaultfolder and clear your browser cache. - Create at least one of each element (not including Category) using the contextual quick create feature. Then, use quick edit on that each element (or others you may have created in your test environment). Try the QCE feature from different pages within the manager to verify the window/form presentation is consistent and labelling and descriptions remain visible.
Special Note
Ultimately, for the TV QCE window, more work will need to be done in a separate PR to bring the tv type-specific field display of the main TV panel to the associated QCE window (for the default value and input options fields). Also, I am aware that changing the window to a modal prevents the ability to drag/drop tree items into the window's fields. IMO, that's not particularly useful in the QCE windows and is problematic from my testing; it's very easy to unintentionally drop an item in the form beneath without knowing it. I've also found it very distracting to have the front window and the main page visually on the same plane.
@smg6511 I do like these changes, except setting the window mode to modal. This mode disallows you to open multiple windows - not sure how others, but I personally have several elements opened in windows at the same time...
As it stands, the quick create/edit (QCE) forms get visually lost against the main MODx interface below. Changing the window to a modal allows better focus on the task at hand.
You can call 2 windows, or work simultaneously with the edit form and the quick window (for example, when I call the snippet, I write the names of the chunks, and in parallel, I create these chunks through QCE), this will not work with a modal window. Yes, visually QCE is lost when called, but this is not a huge problem, it might be worth changing the shadow styles for QCE.
Makes the QCE format more consistent with the associated main editing panels.
Disagree, the interface is now more inconsistent.
- Why are some of the windows in the new "Modal" mode (Template, Chunk, TV), and all the rest in the old (Resource, Plugin, Category, Lexicons, Settings, etc.)? You create 2 different behaviors for the same action - this is bad for the UX. It looks unfinished.
- Descriptions of fields are in QCE only for TV, for others quick windows they are not, and now the description for "Empty Cache" only looks strange. Those it is more correct to remove descriptions if we are talking about consistency.
And the global question: are quick windows so necessary in changes that you touch as many as 1000 lines of code? I am 100% sure that if the PR is merged, then in the future we will catch bugs.
This mode disallows you to open multiple windows
Ok, fair enough. I knew the use of modal may be questionable for some. I don't think I've ever thought about opening multiple QCEs when working in a site. This is where it's important to get the reaction of others who have different work styles ;-) . Perhaps a more elegant interface for these quick editors could be conceived in the future. I'll play with trying to create separation in a different way.
Disagree, the interface is now more inconsistent
Yes, right at this moment. I would carry through whatever changes get accepted here to all windows as well. I am trying to appease the request to have smaller, more easily mergeable PRs by not doing it all in one swoop here.
On the whole, the descriptions of fields should show if inline_help is set to true. I find it really odd that, for instance, tooltips show for Resources no matter what and many areas such as the QCEs show no descriptions. To me that help setting should include these options (and it should apply everywhere):
- off
- inline
- tooltip
I’m not suggesting we change the help setting options in this PR, just that it’s a way that makes more sense to me, and could be considered for future implementation.
Descriptions of fields are in QCE only for TV
You should see them for all element windows in all but the top row of fields (which I debated adding; I probably should for full consistency). At any rate, I had a lot of trouble with the MODx cache not fully clearing when I was making and reviewing these changes. Sometimes I had to navigate away from where I was and clear the cache more than once.
Lastly, the concern about touching many lines of code and bugs is less than you might think with this IMO. I'm not touching any code affecting how these windows process their data, it's all presentational. And, no window JS classes extend these already extending (MODx.Window) classes (they're the final in the chain).
No offense, but your PR looks like this to me:
- Worsening UX by changing the window to modal.
- Worsening UI NOW to bring UI to a single format IN THE FUTURE (but we not sure about that).
- Add unnecessary lexicons (in MODX 2.x everything is clear without them), which adds work to translators.
- We will touch upon a lot of code for improvement that is not critical and not required, but can create bugs. What about the golden IT rule: "If it works, don't touch it"? :)
There are a lot of objective problems in MODX, maybe it's worth fixing them? I'm not trying to tell you what to do, but there are few active participants in the MODX repository and I feel sorry for your efforts and time.
@Ruslan-Aleev - A couple important things re your comment: I think you misunderstood my previous reply (or I wasn't completely clear) —
- I'd conceded that the modal might be an issue and I’d try to find another way to create separation. Even so, I maintain that the readability as implemented is just not good — and that is arguably just as bad UX-wise.
- I need to know that the changes here are going to be accepted/desired (if not merged) before carrying them through to all the window classes. I think it goes without saying that, assuming acceptance of the current set of changes, I would intend on completing the rest of the windows soon after and before any public release. Again, as I pointed out, I was also trying to respond to the past requests of yours and others to make smaller PRs.
On the lexicons issue, I understand that there's not an army of people working on MODx. But shouldn't we be striving for excellence and completeness in the interface? I know you’ve been trying to streamline the lex's to make it easier to maintain, but I think prioritizing that makes for an interface that lacks the polish it could and, IMO, should have. To me, ahead of this 3.x release is the perfect time to try to hone the basics with the lex's and in other areas; make the keying and naming conventions as consistent as possible, make the labelling, descriptions, and other messaging more specific to their contexts. Using say, a date TV as an example: “Default Date” is just more friendly and communicates immediately, as opposed to “Default Value” which is more programmer speak than natural language. Yes you can figure out what should go in that field, but honestly, which is better?
Finally, don't get me wrong — I am an avid proponent of MODx and have been for years — but the more I look, the more I see a significant number of problems that deserve attention (regardless of whether or not they are highly visible or have made it to the issues list). And many of my PRs set out to solve an issue in the list, but balloon into much more because other issues become apparent along the way. Again, I get that this is an open source project with limited resources. But glossing over details too much to push things ahead should be avoided.
Thanks for all the work you've put into this @smg6511 ! I don't see it hampering UX if the changes are uniform for other windows.
Personally I'm fine with the changes... except for a few points. :wink:
- The modal setting does certainly make it look better, but being able have multiple open at once is clearly important. It's also handy to be able to edit a form in the background while having a window open sometimes. We do need a way (I'm not sure how) to add more contrast between an open window and the background.
- The placement of the input elements in the bottom half of the window looks a bit messy. I think this is what @Ruslan-Aleev was talking about when he said it looks unfinished. I do understand why you've made the input option values field longer, though I wish the layout felt a bit more even/cohesive. What about switching the default value field with the empty cache switch, and making the default value field a textarea again?
- In regards to the inline_help text showing below the fields or as tooltips, in a perfect world it'd be nice to have tooltips as the default with a toggle button in the titlebar to dynamically show the text under the fields or not. Again, I'm not sure how easy that would be without breaking BC.
@muzzwood - Thanks for your feedback! Just to explain the logic of why I chose to lay out the TV window in the way you see (not to say I won't change it if folks don't like it):
- It mirrors for the most part the way the fields appear in the main editing panel
- Because probably 99% of the time the Default Value will be short in length, it's field width is shorter
- The Input Options value may likely have a bound query or some other longer text length, which would be easier to deal with in a full-length field IMO
FYI, both the Default Value and Input Options fields are textareas, they're just initially one line in height to conserve space. They're both set to grow vertically to a defined maximum.
On the help text, there's some brainstorming to be done there and a more consistent and elegant solution to be had. I've thought of something somewhat similar to what you've suggested, but it's a feature best worked out in a different proposal/PR.
A couple things for all to note:
- Although I do work on a large screen some of the time, I spend 50%+ of the time working in MODx and other coding/CMS type stuff on a laptop. So I come at this from the perspective of trying to make the layouts work well on a small-ish screen, as well as desktops and mobile.
- Also, I've spent more of my (early) career on the graphic design end of things, so I'm definitely sensitive to the visual balance issues that get pointed out; it can be tricky sometimes getting the desirable design choice to line up with the logical UX choice. And I realize sometimes I'll make a choice that ends up not being the best one for most, and needs to be changed (like the initial choice of changing to modal windows in this PR). Know that I probably will debate reviewers over some choices, but ultimately will bend to whatever the consensus is.
We do need a way (I'm not sure how) to add more contrast between an open window and the background.
We need a sort of a click through layer (CSS: pointer-events: none;) but only added once. It is important that the other parts of the user interface are still accessible, at least for copy & paste values.
@Jako - Thanks for the hint! I took a quick look and should be able to work something out using the strategy you mentioned.
@smg6511 I hope a modal background is just added once in the DOM and there are no other disabling strategies executed in ExtJS when modal: true is set.
@Jako - Yes, I've already sorted out how to do it with just one overlay; it doesn't use the modal mode, as it would've been more complicated to override the associated methods in the Window class and we want to retain the established modal behavior in place for dialogs and message windows.
Here's a quick recording of the custom 'modal' feature (I'm calling these pseudo modals) in action. Thanks again @Jako for the pointer-events suggestion (of which I wasn't aware of since I've not come across the need for it until now).
https://user-images.githubusercontent.com/689075/148699548-0af72a3d-89c8-4ceb-a404-7327fcd3ba43.mov
@smg6511 And someone else besides you is confused by the style of the quick window? Maybe you should create a poll in the community https://community.modx.com/?
Visually love this a lot. From an accessibility perspective, I think it bears a small "Remove mask" button so everyone can more easily read the contents of the left tree by removing the grey background overlay. Awesome job!
This looks fantastic @smg6511 ! So perhaps there could be two small buttons in the title bar. One for toggling the mask, and one for toggling the help text. What do you think?
@smg6511 I'm not sure if showing the darker overlay while allowing the interactions on the background is a good idea. You'll have same UI for modals where you can interact with background objects and for modals where you can't - which might confuse people that are used to not beein able to interact with background objects when the dark overlay is present.
So until you'll try, you won't know if you can or can't do stuff in background.
I'd vote for keeping the dark overlay only for modals where you can't do any interactions with background objects.
@theboxer - I take your point about the overlay color (if they are the same for all modal-type windows). A couple of things to note:
- In the end, if we were to implement what I've done (or something like it), the only modals that block background interaction would be dialogs (confirms, alerts, errors, etc.). Because a user is only tasked to/able to do one thing with these windows, I'd argue that the increased visual separation of them and the main interface is not as useful as it is for those where more work is taking place (in particular the element and resource QCE windows).
- Even though I think, given what I said above, that the nature of the windows would be a clear enough indication of whether you could interact with the background or not, I could make it so blocking modals (or overlays such as those over a disabled grid) retain the white overlay they have now (maybe a little more opaque though), while the other ones maintain the darkened overlay I've set up.
The above said, and with other feedback taken into account (such as adding the ability to disable the overlays, or otherwise control them), I think we could arrive at an overall better UI with the windows that everyone could be happy with.
@rthrash - Do you think it'd be more useful to have the ability to turn off the overlays at the page/window level or should we simply add a system setting where you globally turn them on/off. Also, we could make modal overlay opacity user-customizable in a system setting ... would that be desirable?
@Jako - Given my what I said above (real modals vs. others), I don't think it'd make sense to be able to remove a blocking (real) modal's overlay as there's no reason (or ability for that matter) to interact with the interface below.
@smg6511 how it will work for windows that are using the modal: true setting?
Nothing changes for windows set to modal: true. What I've done is added a new boolean config option pseudoModal to avoid breaking anything the MODx core doesn't have control over (extras) and allow extras to adopt the new feature at will.
@rthrash - Do you think it'd be more useful to have the ability to turn off the overlays at the page/window level or should we simply add a system setting where you globally turn them on/off. Also, we could make modal overlay opacity user-customizable in a system setting ... would that be desirable?
It should be a global setting for sure, with an opacity setting as a nice to have. I would be nice to be able to have a button on pseudo-modals for disabling on pages where they're used, but I'm probably a 5/10 on that front.
Guys, I wanted to pass this proposed layout change by you for windows having the "Empty Cache" switch; see below.
I've placed the switch in the window’s footer bar to gain a bit more space and neaten the form a bit. Also, now that I realize that this cache setting is not persistent (not part of any db record) and is applied on a save-by-save basis, it makes more logical sense for it to be grouped with the action buttons.
An aside on the Clear Cache option: I'm not proposing a change here at this time, but it occurs to me that it'd make more sense (at least to me) for the switch to be "Preserve Cache on Save" with it being off by default. This results in effectively the same default behavior, just advertised to the user differently. Curious what you all think.
Also, a technical detail I'd like opinion on: Unless there's a known special reason for it, IMO the inline help components should be of xtype box rather than label/hidden, as the created element more accurately reflects its contents. The change would look like this, going from:
xtype: MODx.expandHelp ? 'label' : 'hidden',
forId: `modx-${this.ident}-field`,
html: _('field_desc'),
cls: 'desc-under'
to:
xtype: 'box',
hidden: MODx.expandHelp ? false : true,
html: _('field_desc'),
cls: 'desc-under'
Lastly, I'm wondering for the TV QCE window whether it'd be better to provide a tabbed form like the QCE Resource window does. Do you all tend to set up input options and default values a lot when working in these windows?
That's it for now. I'll be working on a couple of the other requests in this conversation before pushing new changes.
I would group the empty cache switch with the close/save buttons on the right (maybe it can be positioned above those buttons). But that't not important.
Judging by the quantity of commented code and log statements this isn't ready for review?
@Mark-H - Let me look through it again. If I remember, I was looking for buy-in early because the changes proposed were major. I think I'd gotten enough feedback to make some decisions. Originally, I was looking for review, but not necessarily approval and merging as-is. I'll either change this to WIP or clean it up for real review in the next few days.
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Jim Graham. This is most likely caused by a git client misconfiguration; please make sure to:
- check if your git client is configured with an email to sign commits
git config --list | grep email - If not, set it up using
git config --global user.email [email protected] - Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails
![cla-bot[bot] avatar](https://avatars.githubusercontent.com/in/2404?v=4 "Published by a pub.dev verified publisher"){kind=small}