Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

entities layouts: ease external ids links customization #464

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

maxlath
Copy link
Member

@maxlath maxlath commented Jan 12, 2024

As we now have plenty of links from external ids to display, having good interfaces to customize what should be displayed seems important. In particular, a user that doesn't explore the settings might not realize that they can display all those links.

type="checkbox"
checked={selectedBibliographicDatabasesCount === bibliographicDatabasesNames.length}
indeterminate={selectedBibliographicDatabasesCount !== 0 && selectedBibliographicDatabasesCount !== bibliographicDatabasesNames.length}
on:click={() => toggleAllSection({ names: bibliographicDatabasesNames, selectedCount: selectedBibliographicDatabasesCount })}
Copy link
Contributor

Choose a reason for hiding this comment

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

I recall a conversation where you where against this checkbox-that-modify-everything behavior. The reason behind was that it is not very accessible, as what is going to happen after clicking on the checkbox is left unknown to the user. Usecase: one crafts their ids customization, then discovers the general checkbox, and on their first click on it, they deletes their own customisation for good.

Select All button pattern seems to be the accepted solution since inventory browser implementation. And is now in other parts of the app (ie. items map, entity homonym selections).
2024-01-14_12-24

So, i think it would make sense, on a less central display than inventory browser (where space is less compact/important), to reuse this pattern, wouldnt it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem with the inventory browser is that the selection can be much bigger than what is on the screen (ex: "select all" might select 1000 items, while only 20 are displayed), so I think my reasoning was that in absence of visual feedback, it was better to make explicit buttons. But in the present case, as we display all the options, I like the minimalism of that single global checkbox: you can still manually select all or none without using it, but it respects a known convention which users might discover, and have direct confirmation that it has the effect they expect.

Copy link
Contributor

@jum-s jum-s Feb 9, 2024

Choose a reason for hiding this comment

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

it respects a known convention

I couln't find this convention anywhere in the codebase though. On the contrary the "select all" pattern appears in more than the inventory browser (ie. map filters)

I would still be more comfortable with an already in use pattern (and more accessible (cf. my last comment)) to not have different display with same behavior.

If you want to continue with this unnamed checkbox, can the input element be extracted into its own component (to make it reusable for map filters) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I couln't find this convention anywhere in the codebase though

I meant a known convention in general, but that has indeed not been used so far in the codebase

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I dont know how to continue this, i think its a mistake to present two different ways to display the same behavior. And sure, its minimalistic, but also more difficult to understand whats going on. This app is already complicated enough to make the user guess what will happen if checking the box.

You seem definitive, so i will swallow it, can we at least extract it in its own component to reuse it elsewhere ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants