modxcms / revolution

MODX Revolution - Content Management Framework
https://modx.com/
GNU General Public License v2.0
1.36k stars 529 forks source link

MODX 3 alpha1 - icons visable not actions #14929

Open OlegShchavelev opened 4 years ago

OlegShchavelev commented 4 years ago

Bug report

The navigation icon is displayed if there is no context menu.

изображение

Environment

MODX version, apache/nginx and version, mysql version, browser, etc. Any relevant information.

Ruslan-Aleev commented 4 years ago

The list of package versions does have a context menu - only the most recent version listed does not. So removing the icon is not a proper fix here.

@Mark-H Perhaps we can close this issue? Because there is, in fact, no bug.

Although the components, it seems to me, are more often updated than not and maybe it’s worth hiding the gear-icon? Not sure.

JoshuaLuckers commented 4 years ago

We have to investigate whether it is possible to hide/show the gear-icon per row and maybe even what "actions" it shows. For packages that don't have any actions we should hide the gear-icon.

Ibochkarev commented 4 years ago

The designated problem remains relevant. Tested on the latest version MODX Revolution 3.0.0-alpha3 (git)

image

smg6511 commented 2 years ago

This is still quite relevant and there are actually issues in quite a few places, in terms of menu options, buttons, and the context and actions menus not accurately reflecting what one has permissions to do. I've been working on this over the past few days now and will submit a PR in the near future.

@JoshuaLuckers - I've figured out how to handle the actions (gear icon) menu visibility on a per-row basis. But in addition to that issue, none of the other avenues of editing/creating (the column-level editors, the create buttons, the batch [delete] buttons, etc.) are in sync with the permissions level a user has. So I'm sifting through that.

The real tricky thing is in the Roles grid: We have two (what I'm calling) protected options (Super User and Member) that should never be editable, whereas the editability of rest of the grid's options is permissions-dependent. There's no built-in way to enable/disable the editor(s) on a row-by-row basis, so that's proving to require a creative solution—one that's in progress.

Anyway, I do have a question for those who might review this: I think it'd be much more user-friendly to have the column editor activate on one click instead of two. To me, when one hovers over a cell and see's the pencil icon, they're expecting something to happen when they click. It's a simple config change, not hard to do. @Ruslan-Aleev - I'd like to get your opinion on this as well, as I know you're very UI-focused.

Ruslan-Aleev commented 2 years ago

Anyway, I do have a question for those who might review this: I think it'd be much more user-friendly to have the column editor activate on one click instead of two.

Yes, the number of clicks is confusing, we discussed this in PR https://github.com/modxcms/revolution/pull/14748#issuecomment-534146728 But with the current implementation, this cannot be quickly fixed, since the pencil icon is only a visual indication that the grid fields are editable. If we make the fields editable by 1 click, then users will often activate the editing by mistake. In general, ideally, we need to make the icon a separate button, and not just an indication.

smg6511 commented 2 years ago

If we make the fields editable by 1 click, then users will often activate the editing by mistake.

The funny thing is, for me because the pen icon appears, I expect to be able to edit upon one click (anywhere on the column and not necessarily a click on the pen icon itself). It'd be interesting to know what others expect too. I'm learning that we have very different expectations on this particular UI element (icons that appear as a visual cue on hover)—I don't expect the icons to be the trigger (unless another visual cue occurs when hovering over that icon). In the case of these editable columns, perhaps the icon is just overkill as we already provide the cue that the field can be manipulated by changing the cursor on hover. The thing we can probably agree on here is that there's likely a learning curve of a handful of clicks for either strategy to know its behavior (the current one I'm very familiar with as I've used MODX since the pre-Revo days)—I guess it's just occurred to me while working on this issue that it really is annoying (to me) to have to click twice to edit.

@Mark-H - It'd be good to get your take on this as well (I see the PR that @Ruslan-Aleev refers to was yours and relates to exactly what I'm asking about).

Mark-H commented 2 years ago

Cases like this I'll happily defer to others with more practical UX knowledge to reach consensus. I can see benefits to each approach of single or double click.

Primary reason it is double click now is because that's how the grid worked before. The PR only added some emphasis and the pen icon to indicate what fields are in fact editable.

smg6511 commented 2 years ago

Considering there may not be strong opinion toward one way or the other, perhaps we put it in the user's control and add a system setting to choose single or double click. That could appease individual preferences, and we just keep the default behavior as-is.

Ruslan-Aleev commented 2 years ago

@smg6511 In general, it is worth making a separate issue, since the discussion of the pencil icon does not apply to this issue of context menu icons. The current pencil icon improves UX (previously it was not clear that the field in the grid could be edited), but there are always better solution :)

smg6511 commented 2 years ago

@Ruslan-Aleev True, I'll leave this question to another possible PR. Maybe I'll propose the single/double-click setting in a new issue (as a feature). FWIW, I don't dislike the use of the pencil icon in general, I'm just not crazy about getting that cue, the cursor cue, and having to double-click to edit a given field. ;-)

rthrash commented 2 years ago

This issue has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/need-insight-on-acl-implementation-in-back-end/4623/1