[3.x] Refine Quick Create/Edit Windows for Elements #15953

smg6511 · 2022-01-05T05:25:39Z

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 build from within the _build/templates/default folder 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.

theboxer · 2022-01-05T12:13:45Z

@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...

Ruslan-Aleev · 2022-01-05T13:57:11Z

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.

smg6511 · 2022-01-05T16:25:54Z

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).

Ruslan-Aleev · 2022-01-05T23:01:51Z

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.

smg6511 · 2022-01-06T05:43:44Z

@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.

muzzwood · 2022-01-06T06:30:09Z

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. 😉

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.

smg6511 · 2022-01-06T16:18:00Z

@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.

Jako · 2022-01-06T16:29:00Z

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.

smg6511 · 2022-01-06T19:42:33Z

@Jako - Thanks for the hint! I took a quick look and should be able to work something out using the strategy you mentioned.

Jako · 2022-01-07T10:04:34Z

@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.

smg6511 · 2022-01-07T18:39:13Z

@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.

smg6511 · 2022-01-09T20:28:21Z

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).

modx-qce-pseudoModals-ex3-720.mov

Ruslan-Aleev · 2022-01-09T21:42:40Z

@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/?

rthrash · 2022-01-09T22:55:36Z

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!

Jako · 2022-01-09T23:17:43Z

A 'remove mask' is also an option for the 'modal' background. Nice idea!

muzzwood · 2022-01-10T01:15:18Z

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?

theboxer · 2022-01-10T10:29:29Z

@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.

smg6511 · 2022-01-10T19:37:58Z

@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.

theboxer · 2022-01-10T19:40:52Z

@smg6511 how it will work for windows that are using the modal: true setting?

smg6511 · 2022-01-10T19:48:04Z

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.

theboxer · 2022-01-10T19:49:08Z

👍

rthrash · 2022-01-11T21:18:11Z

@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.

smg6511 · 2022-01-15T17:37:47Z

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.

JoshuaLuckers · 2022-01-31T08:15:53Z

I never use the quick create TV option.

Jako · 2022-04-04T08:36:20Z

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.

Mark-H · 2023-01-30T19:25:46Z

manager/assets/modext/widgets/core/modx.panel.js

-     * @param {Object} cmp - The help field's Ext.Component object
-     * @param {String} elType - The MODX element type (i.e., tv, chunk, or snippet)
-     */
-    ,insertTagCopyUtility: function(cmp, elType) {


Moving this looks like a breaking change. Consider keeping the method but making it pass through to MODx.util.insertTagCopyUtility.

Mark-H · 2023-01-30T19:26:52Z

manager/assets/modext/widgets/core/modx.window.js

-        this.resizeWindow();
+    this.on({
+        render: function() {
+            console.log('window render, this:', this);


stray window.log

Mark-H · 2023-01-30T19:27:31Z

manager/assets/modext/widgets/core/modx.window.js

+                    // console.log('open modxPseudoModals: ',MODx.openPseudoModals);
+                }
+                if (this.modal) {
+                    console.log('rendering real modal...');


stray log, or something that still needs to be implemented? Should the pseudo modal handling just not run if this.modal is set?

Mark-H · 2023-01-30T19:28:37Z

manager/assets/modext/widgets/core/modx.window.js

+            });
+
+        }
+        console.log('final fields: ', this.config.fields);


stray log and a bunch of commented out code below that can be removed (or is that still needed to be implemented?)

Mark-H · 2023-01-30T19:29:29Z

Judging by the quantity of commented code and log statements this isn't ready for review?

smg6511 · 2023-01-30T19:33:06Z

@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.

Post-rebase fixes and and updated modal behavior in response to reviewer feedback.

cla-bot · 2023-09-23T05:22:57Z

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@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

smg6511 requested review from Mark-H and opengeek as code owners January 5, 2022 05:25

cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Jan 5, 2022

Mark-H added this to the v3.1.0 milestone Jan 19, 2022

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from a79b8a2 to 45d2bab Compare January 31, 2022 05:27

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from 648b19a to ec33674 Compare March 24, 2022 03:29

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from ec33674 to e2a55e7 Compare April 3, 2022 15:41

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from e2a55e7 to bc31a0c Compare May 6, 2022 06:58

smg6511 added the in-progress PRs that are in progress by the author or contributors label May 6, 2022

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from 76f0e3a to a8e7e31 Compare July 17, 2022 04:47

Ibochkarev added pr/review-needed Pull request requires review and testing. and removed in-progress PRs that are in progress by the author or contributors labels Aug 10, 2022

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from a8e7e31 to 24bc380 Compare September 17, 2022 05:16

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from 24bc380 to 8ec573a Compare November 4, 2022 15:44

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from 8ec573a to cbe37b6 Compare January 14, 2023 04:32

Mark-H reviewed Jan 30, 2023

View reviewed changes

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from cbe37b6 to 808fe41 Compare June 10, 2023 03:51

codecov bot deleted a comment from codecov-commenter Jun 10, 2023

Jim Graham added 4 commits September 23, 2023 01:20

Quick Create/Edit Window Refinements

cad05c1

Update _forms.scss

838e322

Progress changes

22e906c

Post-rebase fixes and and updated modal behavior in response to reviewer feedback.

Fix errors found via Lexicon unit test

1933ff4

smg6511 force-pushed the 3.x-refine-qce-windows--tv branch from 808fe41 to 1933ff4 Compare September 23, 2023 05:22

cla-bot bot removed the cla-signed CLA confirmed for contributors to this PR. label Sep 23, 2023

Mark-H added blocked Active participation around the pull request or issue required. Consensus is not reached. and removed pr/review-needed Pull request requires review and testing. labels Feb 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3.x] Refine Quick Create/Edit Windows for Elements #15953

[3.x] Refine Quick Create/Edit Windows for Elements #15953

smg6511 commented Jan 5, 2022 •

edited

theboxer commented Jan 5, 2022

Ruslan-Aleev commented Jan 5, 2022

smg6511 commented Jan 5, 2022

Ruslan-Aleev commented Jan 5, 2022

smg6511 commented Jan 6, 2022

muzzwood commented Jan 6, 2022

smg6511 commented Jan 6, 2022 •

edited

Jako commented Jan 6, 2022 •

edited

smg6511 commented Jan 6, 2022

Jako commented Jan 7, 2022

smg6511 commented Jan 7, 2022 •

edited

smg6511 commented Jan 9, 2022

Ruslan-Aleev commented Jan 9, 2022

rthrash commented Jan 9, 2022

Jako commented Jan 9, 2022 •

edited

muzzwood commented Jan 10, 2022

theboxer commented Jan 10, 2022

smg6511 commented Jan 10, 2022 •

edited

theboxer commented Jan 10, 2022

smg6511 commented Jan 10, 2022 •

edited

theboxer commented Jan 10, 2022

rthrash commented Jan 11, 2022

smg6511 commented Jan 15, 2022 •

edited

JoshuaLuckers commented Jan 31, 2022

Jako commented Apr 4, 2022

Mark-H Jan 30, 2023

Mark-H Jan 30, 2023

Mark-H Jan 30, 2023

Mark-H Jan 30, 2023

Mark-H commented Jan 30, 2023

smg6511 commented Jan 30, 2023

cla-bot bot commented Sep 23, 2023

[3.x] Refine Quick Create/Edit Windows for Elements #15953

Are you sure you want to change the base?

[3.x] Refine Quick Create/Edit Windows for Elements #15953

Conversation

smg6511 commented Jan 5, 2022 • edited

What does it do?

Why is it needed?

How to Test

Special Note

theboxer commented Jan 5, 2022

Ruslan-Aleev commented Jan 5, 2022

smg6511 commented Jan 5, 2022

Ruslan-Aleev commented Jan 5, 2022

smg6511 commented Jan 6, 2022

muzzwood commented Jan 6, 2022

smg6511 commented Jan 6, 2022 • edited

Jako commented Jan 6, 2022 • edited

smg6511 commented Jan 6, 2022

Jako commented Jan 7, 2022

smg6511 commented Jan 7, 2022 • edited

smg6511 commented Jan 9, 2022

Ruslan-Aleev commented Jan 9, 2022

rthrash commented Jan 9, 2022

Jako commented Jan 9, 2022 • edited

muzzwood commented Jan 10, 2022

theboxer commented Jan 10, 2022

smg6511 commented Jan 10, 2022 • edited

theboxer commented Jan 10, 2022

smg6511 commented Jan 10, 2022 • edited

theboxer commented Jan 10, 2022

rthrash commented Jan 11, 2022

smg6511 commented Jan 15, 2022 • edited

JoshuaLuckers commented Jan 31, 2022

Jako commented Apr 4, 2022

Mark-H Jan 30, 2023

Choose a reason for hiding this comment

Mark-H Jan 30, 2023

Choose a reason for hiding this comment

Mark-H Jan 30, 2023

Choose a reason for hiding this comment

Mark-H Jan 30, 2023

Choose a reason for hiding this comment

Mark-H commented Jan 30, 2023

smg6511 commented Jan 30, 2023

cla-bot bot commented Sep 23, 2023

smg6511 commented Jan 5, 2022 •

edited

smg6511 commented Jan 6, 2022 •

edited

Jako commented Jan 6, 2022 •

edited

smg6511 commented Jan 7, 2022 •

edited

Jako commented Jan 9, 2022 •

edited

smg6511 commented Jan 10, 2022 •

edited

smg6511 commented Jan 10, 2022 •

edited

smg6511 commented Jan 15, 2022 •

edited