[WEB] Fixing UI/UX on /People

[warm ember]

This is a topic to discuss how it would be best to implement/fix this issue https://github.com/immich-app/immich/issues/19956

This is my current proposal:

UI/UX Proposal: Separation of Concerns

To improve clarity, I propose separating Viewing (filtering) from Managing (bulk actions) in the /people section.

1. Visibility Filter (Eye Icon)
   A toggle or dropdown to control the current view state:

• Standard: Show unhidden people only.
• Hidden Only: Show only hidden people.
• Show All: Display everyone.

2. Management Menu (Bulk Actions)
   Replace the current "Show & Hide" button with a "Manage Visibility" inside, the current eye icon would become a dropdown for batch updates:

• Invert Shown/Hidden: Swap the status of all entries.
• Hide Anonymous: Set all unnamed people to "Hidden".
• Hide Identified: Set all named people to "Hidden".
• Hide All: Set all people to "Hidden".
• Reveal Anonymous: Set all unnamed people to "Shown."
• Reveal Identified: Set all named people to "Shown."
• Reveal All: Set all people to "Shown."

I do believe that now we had a bit of an options explosion, as I'm considering all possible options a person would have in the management section.

given these are just the "oposite" of the other, maybe we could make em work kind of like a toggle? so if all "annonymous" were already hidden, it would Unhide all of the annonymous ones, and vice versa. same would apply for the other options

so it would end up becoming

• Invert Shown/Hidden
• Hide/Show Anonymous
• Hide/Show Identified
• Hide/Show All

4 options in a dropdown.

[uneven rampart]

I'm against 3 state toggles, I don't find then user friendly.
IMHO from the usability perspective, it would be better a filter option with checkbox (remembering last state) and a button to apply/remove the filtering.
but is not a feature I use anyway, so there are probably better suggestions 😬

[warm ember]

IT isn't 3 toggles per say, but more the same button doing different things based on the case.

But for now we could leave it explicit with all the options and then improve that further

the filter isn't the same as the managing part

[woeful patrol]

A cool feature would be to be able to sort the people by "recently identified" and also by "current popularity" (people who show up often recently)

[warm ember]

Thats cool and all for the filtering on the outside! but I think its getting a little out of scope- I was just trying to fix the issue complained about.

but I'd be down to making that a filter in the outside as well if the team agrees on it but I find that to be a "feature request" rather than a bug fixing for now

so maybe for the future

the issue I''m trying to address right now is this https://github.com/immich-app/immich/issues/19956

[swift badger]

[Issue] [web] Toggle person visibility actually changes hidden state (immich-app/immich#19956)

[gentle fulcrum]

I don't think this can even be reliably put in because AFAIK there is no timestamp linked to a person's creation

[muted crane]

idk but wouldnt it be possible to track and extract that info from logs?(maybe docker logs?) , i didnt read the docker recognition logs too carefully(just skimmed) but i think there was some values (like person 1834365 sort of values which is probably connected to the persons?) which can be used?
(this assumes the logs are never deleted prob?)

[gentle fulcrum]

That's some nice out of the box thinking but logs are transient and in no way, shape or form does the server process monitor it's own logs.

[muted crane]

wouldnt something like import this file (log file) slot for extracting proper person identification values is possible?

via manual user help

[gentle fulcrum]

Again, nice out of the box thinking but

1. User creation tracking won't be implemented in this discussion/topic and
2. Definitely won't be implemented with some weird log follow/import/whatever thing

If it was on the table it would be done with database edits the moment a person is created

[muted crane]

shall i erase these btw?(as theyre out of topic)

[gentle fulcrum]

Nah

[muted crane]

just to confirm , how would we hide only few of the named peoples or show some of the unnamed peoples? (i can imagine thered be potencial use cases , like a photographer sorting the job/faces?(done/not done at basic level))

[warm ember]

well- you'd be able to "hide all" and then unhide only the ones you want for example.

[warm ember]

thanks for keeping it in topic Mraedis! :p

I'll leave this open in the meantime for more feedback about this. hopefully. and then send a formal message on github, to then move on to building the pr.

[molten glen]

I'm having a hard time understanding why you'd want to hide a group. Would "show only" be an alternative option? I guess that brings us pretty close to current behavior, just listed out instead of being icon states

[warm ember]

sorry, what did you mean by this exactly? dont think I got it haha

[molten glen]

How is Hide Anonymous different from Show only identified?

[warm ember]

one is a filter, the other is actions

thats what the whole issue is about, separating concerns to make UI more easy to deal with

[molten glen]

Ah. My apologies for misunderstanding

[pure idol]

@warm ember first of all, great that you want to take care of this issue. I was the one creating it 😉

In the first place the issue is about the expectation what it might do and what it actually does. As per my understanding the whole UI did not appear to have anything to do with bulk manage of hidden states.

So from my point of view fixing this would mean, getting rid of the bulk update.

But I like your proposal about the option to separate viewing different sets of people depening of their hidden/named state and managing the afore mentioned sets of peaple in a bulk manner

[warm ember]

I've already done something on my own fork :p hopefully that will fix it!

[pure idol]

did you see https://github.com/immich-app/immich/pull/26815 ?

[swift badger]

[Pull Request] fix(web): toggle visibility button filters view instead of changing state (immich-app/immich#26815)

[warm ember]

Uuuuh... no? O-oh... have I been snipped?

I cant submit until the university allows me to finally do my PR >.<

his submission also has no tests...

[gentle fulcrum]

It's new, it's not merged, no worries

... pretty sure it's AI too

[warm ember]

okay...

gave me a heart attack

lol

had to essentially change my commit message 2 times until it eventually got accepted by our system lol. so the same commit was referenced 3 times in the issue

[warm ember]

In the end. I figured out how to make the labels show, and that was satisfactory already. In the bulk manage section

then I also added the filtering option (no db changes). the whole flow seems to be much more intuitive. Am excited to see what you guys will think of it after we're allowed to submit the PR. if anyone's curious they can also check the referenced commit. But once again, it is in no way final/ready until the university says so haha

[warm ember]

Pull request submitted 🎉

[gentle fulcrum]

Do you have a deadline for the PR review @warm ember because it can take a while 😛

[warm ember]

naaah, its all good!

I'll just have to ping every once in a while if necessary and reply in less than a week to any questions :V according to the guidelines lol

[fierce bear]

Two persons listet, given the same name by user, given the same date of birth by user - should be „auto merged „ to one person 😊

[frosty storm]

That already happens. If you have multiple people with names already and you give a new person a name that already exists, Immich will ask you if you want to merge them

[warm ember]

Hey @frosty storm, Sooo- I was just wondering, what would you like me to do with the PR? do I make a new PR just with the modal transformed into a new page? or do I update the curent PR?

[frosty storm]

Whichever you prefer :)

[warm ember]

Alrighty.

[warm ember]

You think just the creation of the new page will be enough to close the issue? just wanna make sure I dont go in a wrong direction that results in code getting killed again >.<

[frosty storm]

I would say so

Even if we decide otherwise in the end, we'd definitely want that code regardless. I am promising you that we will merge a PR that migrates the modal to a dedicated page, regardless of any issue

[warm ember]

I got it mostly done I think- If it's not much of a bother, could you say something similar in the PR, so I can submit the changes?

[frosty storm]

What exactly? That you don't have to open a new PR?

[warm ember]

Yeah- just something saying that "this seems like the right way forward" and that I dont have to do a new pr and can just essentially "revert those commits" and add the new one in. Just something that signals a true "go ahead. dont worry too much hahaha. I appreciate the attention :p

[frosty storm]

Wrote something. Does that work?

[warm ember]

yes it does! thank you :3

[frosty storm]

Everything to get you a good grade :P

[warm ember]

Well- Where it is deserved ofc. I wouldnt want a grade that I dont deserve hahaha.

open source contributions are kinda fun

[frosty storm]

So far you're doing great :)

[frosty storm]

It'd be awesome if we'd see you around after this :)

[warm ember]

We'll see, I definitely see a ton of tiny things that could take some attention :p

I gotta get immich running locally sometime as well-

By the way I've already submitted the code. Tried to do what I could, but had to traverse some "uncharted" territory :V

[frosty storm]

That was quick!

I'll give it a review (hopefully) tomorrow :)

[warm ember]

already had it ready, was just running tests once more, never enough testing-

[warm ember]

Thanks a lot for the review, you brought an ak47 with you hahaha

Some very nice feedback tho :p

[frosty storm]

I tried to not make it sound like everything's bad, cause it honestly isn't. Most of my comments are just design choices :)

[warm ember]

Most of the stuff I had copied from the /people/+page to keep the same design.

But there's' something that worries me a little actually

at least when I checked seemed to be "wrong design"

but I just kept it as it was "normal"

"let people = $derived(data.people.people);" this in specific.

What was brought to my attention is that... after you set a variable with derived. concatenating more stuff into it would cause problems. as you cant "concat" to a variable that is derived from something else

I kept it, cause I wasnt certain if it was an actual problem or not, but it was what was done in people/+page

[frosty storm]

Yeah that's fair. I figured since you're already moving it I can also have you do some light refactoring :)

[warm ember]

yeah! I just didnt know if I should be touching that other page as well is all haha.

[frosty storm]

Oh my bad, I missed that it's editing that array later. let is perfectly fine then

[warm ember]

is that "derived" and concat problem a real problem? or am I just halucinating? I couldnt test it immediately I dont think cause I dont have enough people for it to use the pagination loading.

[frosty storm]

I don't understand your question tbh

You can assign to a derived variable

That'l be reactive, too

[warm ember]

So doing concat to a variable that is derived should be fine?

[frosty storm]

Yes

[warm ember]

alright then!

[frosty storm]

(but of course, please test it :D)

[warm ember]

Alright- from testing with other examples in svelte I noticed how it actually works-

if X is derived from Y.

when you do X += "something", it will happly update X however you see fit.

But the moment Y gets changed, all changes done to X are erased, and it just strictly copies the new information from Y over.

[frosty storm]

Yes of course, derived will update the variable if the content changes

But that's not an issue here, is it?

[warm ember]

well it was one of my worries, Cause I was unsure if svelte would straight up throw an error at my face or not haha.

I'm trying to see if there's a better way to do the loading of the pages

[frosty storm]

I'm trying to see if there's a better way to do the loading of the pages
What's bad about the current approach?

[warm ember]

I just was trying to make it clearer.

From my understading we were initially loading the 1st page, and then updating subsequent ones. I was just trying to see if any of the variables could be changed to state from derived, etc.

[frosty storm]

Simplifying state/reactivity is always appreciated. Just be sure to test properly as it's very easy to accidentally miss an edge case and break reactivity :)

[warm ember]

Tried some options, but they all seemed to work a little weirdly, so I mostly focused on all the things you mentioned in the review! I think I've now addressed all of the sections :3

For future reference- How should I handle multiple "review changes"?

As I did? discussing on each of em (when it made sense), and then doing a few commits to tackle them, and then "requesting a review" again using github's button?

Or is there a better way to handle it (and hopefully be less annoying) as well?

[frosty storm]

Commenting on the ones you have questions about and otherwise just implementing them is a good way, yes. Re-requesting the review in the end isn't too important for me personally, since I'll get a notification when you update it anyways, but generally that's a good practice too

As for commits, I typically just keep one commit and ammend that. If there's like completely distinct things, those would go in a separate commit. I like to think of commits as individual packets of the final work, rather than small steps along the way

But, especially for such smaller PRs, it doesn't really matter either way

[warm ember]

Alright! that's fair enough- I mean I also usually enjoy squashing every now and then to keep things concise.

Thanks for mentoring me a lil :p

[warm ember]

Hey @frosty storm :p Hope everything is going well!

I just noticed that there was a major renaming on most of the files at immich, which resulted in some conflicts with my PR. Should I do anything about it? or keep waiting until Alex eventually checks the PR?

[frosty storm]

Definitely keep your PR up to date. We can't merge it if it has conflicts :)

[warm ember]

Alright, just was unsure if I should have changed it or not hehe. but alright!

[warm ember]

Done :3

[warm ember]

🎉 🎉 🎉 🎉 🎉 🎉 🎉

Thanks for all the help and guidance in pushing it through Daniel! :p

[frosty storm]

Thanks for working on it!

[brazen kite]

Also i cant merge people from mobile ui current stable version… am i missing something?