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

feat(web): shared link filters #15948

Merged
merged 1 commit into from
Feb 7, 2025
Merged

feat(web): shared link filters #15948

merged 1 commit into from
Feb 7, 2025

Conversation

jrasm91
Copy link
Contributor

@jrasm91 jrasm91 commented Feb 7, 2025

  • Top level page for shared links
  • Navigate to a specific shared link by url /shared-links/:id
  • View individual shared links vs album shared links using GroupTab
  • Option to show shared links in the sidebar (saved as a user preference on the server)

image

@jrasm91 jrasm91 force-pushed the feat/shared-link-filters branch from f65291f to 154b7db Compare February 7, 2025 17:03
Comment on lines +49 to +51
await removeSharedLink({ id });
notificationController.show({ message: $t('deleted_shared_link'), type: NotificationType.Info });
await refresh();
Copy link
Member

Choose a reason for hiding this comment

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

I think we could already optimistically remove the link from the array if the removeSharedLink call was successful, and then after that actually re-fetch the links. That could make it feel a bit more snappy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I'll leave that for a follow-up though. There's definitely a lot of room for improvement though, this was how it was working previously.

let filters = Object.keys(filterMap);
let labels = Object.values(filterMap);

const getActiveTab = (url: URL) => {
Copy link
Member

Choose a reason for hiding this comment

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

You only use that function in the $derived rune. Idk if that's cleaner, but an option would be to inline it instead with $derived.by(() => {}).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I generally find this more readable

@jrasm91 jrasm91 merged commit c5360e7 into main Feb 7, 2025
36 checks passed
@jrasm91 jrasm91 deleted the feat/shared-link-filters branch February 7, 2025 18:05
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.

2 participants