Skip to content

Support 'selectionMode' on tabular and cards#8010

Merged
rijkvanzanten merged 19 commits intodirectus:mainfrom
joselcvarela:fix-7935
Dec 2, 2021
Merged

Support 'selectionMode' on tabular and cards#8010
rijkvanzanten merged 19 commits intodirectus:mainfrom
joselcvarela:fix-7935

Conversation

@joselcvarela
Copy link
Copy Markdown
Member

@joselcvarela joselcvarela commented Sep 13, 2021

If 'selectionMode'
== 'no-select' : hides checkboxes
== 'select-one': hides select all but show checkboxes (as radio buttons)
== 'select-multiple': show select all and checkboxes

Fixes #7935
Fixes #7818
Fixes #7761 (not using v-radio, but emulating with icons)


Edit:
Fixes #8019

@joselcvarela joselcvarela changed the title Add 'selectionMode' to tabular and cards Support 'selectionMode' on tabular and cards Sep 13, 2021
@joselcvarela
Copy link
Copy Markdown
Member Author

Before:

lrVLQB13qe.mp4

After:

ubWhg7Wp9Y.mp4

@rijkvanzanten
Copy link
Copy Markdown
Member

@joselcvarela we should also figure out how this would work for the Calendar and Map layouts 🤔

@joselcvarela
Copy link
Copy Markdown
Member Author

I didn't find the select all on those layouts. I am still discovering the app, so it's completely normal. I will setup those fields and try to replicate the behavior. But my concern is how the 'selectionMode' is declared. It is ok like that or should I change?

@benhaynes
Copy link
Copy Markdown
Member

Interesting point. I don't think we have a selection mode in Map or Calendar... but we should (need to?). I can work on a design for this.

@benhaynes
Copy link
Copy Markdown
Member

By the way, this is awesome work, @joselcvarela!! ❤️

@rijkvanzanten
Copy link
Copy Markdown
Member

But my concern is how the 'selectionMode' is declared. It is ok like that or should I change?

Yeah looks good to me!

The only thing I would change is that I would call them "one, multiple, none" instead of "select-*" as we're already in the selectMode context 🙂

@joselcvarela
Copy link
Copy Markdown
Member Author

Updated 😉

Copy link
Copy Markdown
Contributor

@nickrum nickrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this PR we have selection, selectMode and selectionMode layout props. This might be a bit confusing, but I can't think of a better name either.

@rijkvanzanten
Copy link
Copy Markdown
Member

selection, selectMode and selectionMode

Yeah we should differentiate between those last two somehow 🤔

@joselcvarela
Copy link
Copy Markdown
Member Author

joselcvarela commented Sep 16, 2021

@nickrum @rijkvanzanten what about selectAmount and use -1 for multiple, 0 for none and then all positive numbers representing a limit of the selection. I think this is not necessary at the moment but can be useful on the future. For example, expecting only 3 items to be selected.

@rijkvanzanten
Copy link
Copy Markdown
Member

@joselcvarela I don't think that necessarily solves for the "selectMode" prop 🤔

"selectMode" tells the layouts that we don't want to navigate to the detail page at all, and only want to select things. This is different from the amount of items that are allowed to be selected. We have to keep in mind that there's cases where you can both select X items and navigate to the detail page, like on the tabular layout

@joselcvarela
Copy link
Copy Markdown
Member Author

@nickrum
@rijkvanzanten

I was referring to selectionMode but I think what you are thinking.
So instead of having this 3 props, we could only have selection and selectionMode.
If selectionMode is:

  • none. Clicking row opens the record and no checkboxes appear
  • one. Clicking row selects it and radio buttons appear
  • multiple. Clicking row selects it and checkboxes appear.

Seems correct?

@rijkvanzanten
Copy link
Copy Markdown
Member

rijkvanzanten commented Sep 16, 2021

Seems correct?

Not entirely! 😁

When you open the collection page for any collection, the table layout allows both clicking the row to navigate, and allows for selection multiple items:

CleanShot 2021-09-16 at 10 30 05@2x

(1) Will navigate to the detail page for the item
(2) Will select the item to enable batch edit/delete etc.

However, when the same layout is used in a relational selection drawer, the interaction change slightly...

CleanShot 2021-09-16 at 10 31 25@2x

Now, both (1) and (2) should select the row. The "one vs multiple" question is also only relevant in that drawer-use

So we'll need to keep a prop for "navigate to detail on click", and a separate prop for "how many items can be selected; none, one, multiple". I think it's mostly a naming question. Maybe we can rename selectMode to navigate (default to true), and have selectMode be none | one | multiple 🤔

@joselcvarela
Copy link
Copy Markdown
Member Author

In my point of view, selectMode and selectionMode could be merged into a single prop called selectMode (like the first one).
This prop could take these as value:

  • 'none': No checkboxes should appear. Clicking on rows should navigate
  • 'readonly': Checkboxes should appear but are not modifiable. Clicking on rows should do nothing
  • 'one': Radio buttons should appear. Clicking on rows should select the current row and deselect previous selected row. Clicking on selecte row should deselect it.
  • 'multiple': Checkboxes should appear. Clicking on rows should select/deselect according they are deselected/selected, respectively.

What do you think?

@rijkvanzanten
Copy link
Copy Markdown
Member

What do you think?

I think that still would have the same problem. For example, the tabular layout by default would have "multiple" and needs to be able to click on a single row to navigate. By having it as a single prop, you can't have "multiple" and "none" from your examples at the same time

@joselcvarela
Copy link
Copy Markdown
Member Author

@rijkvanzanten updated

@joselcvarela
Copy link
Copy Markdown
Member Author

Here's a summary:

7Gjnaw1HCh.mp4

@azrikahar
Copy link
Copy Markdown
Contributor

Update on additional changes I pushed

Question/Discussion

Show we hide the checkbox when the user only has read permission? The user won't be able to actually do anything with it.

eLFNanX8RZ

@joselcvarela
Copy link
Copy Markdown
Member Author

Thanks @azrikahar
I think in the meantime the changes to fix #7935 have been lost.

That's a good question.
A use case of selection while having only read permissions is sharing links.
Since we do not have this feature for now, we can hide it, I think.

@azrikahar
Copy link
Copy Markdown
Contributor

azrikahar commented Nov 12, 2021

No worries 👍 The sharing link is interesting! Maybe leaving it on & showing the checkboxes won't really be a huge deal since I assume a user should understand they only have read permissions anyway. Maybe others can share more thoughts on it 😄

@licitdev
Copy link
Copy Markdown
Member

licitdev commented Nov 12, 2021

Pushed a fix to de-selecting by clicking on a row during selection mode.
The item was not reactive, thus the equality check in filtering does not work.

Screen.Recording.2021-11-12.at.8.04.35.PM.mov

There's a warning about the collection name not being a string. I believe it's something to do with the following code, but am unsure how to go about it.

const defaultValues = {
collection: null,
layout: 'tabular',
search: null,
scope: 'all',
layout_query: null,
layout_options: null,
filter: null,
};

@oreilles
Copy link
Copy Markdown
Contributor

There are some issues with the map selection:

  • Clicking on a point to select it doesn't work (it resets the selection but doesn't select the point)
  • When holding the alt key while doing a selection, if some of the items you want to add are already selected, the whole selection is reset: (in the video the first 5 secs are holding shift, the following are holding alt+shift
Enregistrement.de.l.ecran.2021-11-16.a.12.19.45.mov

I suggest reverting to the previous behavior rules (but open to discuss it)

  • Clicking on a point or doing a box-select should set the selection to only the clicked / hovered items, even if they already are selected
  • Clicking on a point or doing a box-select with alt pressed should add the clicked / hovered items to the selection if they aren't selected yet, and remove them from the selection it they already are.

Nothing to add on the rest of this PR, looks good!

@joselcvarela
Copy link
Copy Markdown
Member Author

  • Clicking on a point or doing a box-select with alt pressed should add the clicked / hovered items to the selection if they aren't selected yet, and remove them from the selection it they already are.

I think this should not be like this. I didn't know how to deselect a point and needed to check the source code to see how I could deselect it. When you see something selected/checked, usually you click on it for deselect.

What are your thoughts @benhaynes @rijkvanzanten ?

@rijkvanzanten
Copy link
Copy Markdown
Member

Good point.. I'm actually not too sure what I would expect to happen 🤔 There's a bit of a conflict here between click to navigate and click to select. That being said, once you've selected one or more items, I would expect further clicks to toggle between the selection state (this would also match the UX from tabular/cards)

@benhaynes
Copy link
Copy Markdown
Member

I'm a little lost in terms of what we're doing now, but here's what I think...

Navigate

  • When not in a selection state, clicking an item will navigate to the item detail page

Select

  • Select Mode: Clicking an item should toggle the individual selection state
  • Select Mode: Dragging over items should select them, never deselect
  • Select Mode: Clicking/dragging outside of items should deselect all of them
  • Select Mode: Holding alt could temporarily enter deselection mode for clicking/dragging items
    • Ideally we also change the cursor so this is clear!
    • If we can't change the cursor (or something else) then this might not be clear enough
  • Normal Mode: Holding alt should temporarily enter selection mode for clicking items
    • Ideally we also change the cursor so this is clear!

Basically, alt temporarily inverts the action. And we could add a note to the screen to tell the users about this keystroke.

Another point, when you drag OVER a point (but haven't released), it would be nice if it changed the style/state of the item dot to show it is about to be selected.

Did I miss anything? This is how most design applications work, and seems to be the most intuitive.

@oreilles
Copy link
Copy Markdown
Contributor

Normal Mode: Holding alt should temporarily enter selection mode for clicking items

OK with these rules, @joselcvarela I can take it from there if that works for you!

@joselcvarela
Copy link
Copy Markdown
Member Author

Yeah, no problem @oreilles 🤝

@rijkvanzanten rijkvanzanten added this to the v9.1.3 milestone Dec 2, 2021
@rijkvanzanten
Copy link
Copy Markdown
Member

It seems like the above mentioned rules by @benhaynes work in this PR (with Shift instead of Alt for mac vs windows interoperability). Seeing this PR fixes numerous bugs elsewhere, lets pick up any optimizations in the selection state of the map layout we'd wanna do in a separate PR.

@rijkvanzanten rijkvanzanten merged commit eb68f85 into directus:main Dec 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

7 participants