Skip to content

chore: Refactor Edit Album Modal #18653

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 29, 2025
Merged

chore: Refactor Edit Album Modal #18653

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df75498
chore: refactor edit-album-modal
arnolicious May 26, 2025
07abe64
chore: fix closing modal
arnolicious May 26, 2025
e05fa0c
chore: refactor to use onClose for success callback
arnolicious May 28, 2025
1c39240
fix: close context menu before opening modal
arnolicious May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 5 additions & 17 deletions web/src/lib/components/album-page/albums-list.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@

let albumGroupOption: string = $state(AlbumGroupBy.None);

let albumToEdit: AlbumResponseDto | null = $state(null);
let albumToShare: AlbumResponseDto | null = $state(null);
let albumToDelete: AlbumResponseDto | null = null;

Expand Down Expand Up @@ -257,8 +256,11 @@
await deleteSelectedAlbum();
};

const handleEdit = (album: AlbumResponseDto) => {
albumToEdit = album;
const handleEdit = async (album: AlbumResponseDto) => {
await modalManager.show(EditAlbumForm, {
album,
onEditSuccess: successEditAlbumInfo,
Copy link
Member

Choose a reason for hiding this comment

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

Just looked at this again and you should get rid of onEditSuccess. EditAlbumForm should have an onClose(album?: ...) => void; if album is provided it was a success, otherwise it was cancelled. That's how we treat those generally now

});
closeAlbumContextMenu();
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to close the context menu before the modal is opened? That's how it was before. (kind of, because before the operation before wasn't blocking)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, tho I hadn't noticed that, because (for me) the context menu closes automatically when clicking one of the options. Even I completely comment out the close function here

Copy link
Member

Choose a reason for hiding this comment

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

Can probably revisit this eventually. For now I don't want to risk running into some random edge cases on some browser due to an unrelated refactor :)

};

Expand Down Expand Up @@ -305,8 +307,6 @@
};

const successEditAlbumInfo = (album: AlbumResponseDto) => {
albumToEdit = null;

notificationController.show({
message: $t('album_info_updated'),
type: NotificationType.Info,
Expand Down Expand Up @@ -422,15 +422,3 @@
<MenuOption icon={mdiDeleteOutline} text={$t('delete')} onClick={() => setAlbumToDelete()} />
{/if}
</RightClickContextMenu>

{#if allowEdit}
<!-- Edit Modal -->
{#if albumToEdit}
<EditAlbumForm
album={albumToEdit}
onEditSuccess={successEditAlbumInfo}
onCancel={() => (albumToEdit = null)}
onClose={() => (albumToEdit = null)}
/>
{/if}
{/if}
5 changes: 2 additions & 3 deletions web/src/lib/components/forms/edit-album-form.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
interface Props {
album: AlbumResponseDto;
onEditSuccess?: ((album: AlbumResponseDto) => unknown) | undefined;
onCancel?: (() => unknown) | undefined;
onClose: () => void;
}

let { album = $bindable(), onEditSuccess = undefined, onCancel = undefined, onClose }: Props = $props();
let { album = $bindable(), onEditSuccess = undefined, onClose }: Props = $props();

let albumName = $state(album.albumName);
let description = $state(album.description);
Expand Down Expand Up @@ -71,7 +70,7 @@

<ModalFooter>
<div class="flex gap-2 w-full">
<Button shape="round" color="secondary" fullWidth onclick={() => onCancel?.()}>{$t('cancel')}</Button>
<Button shape="round" color="secondary" fullWidth onclick={onClose}>{$t('cancel')}</Button>
<Button shape="round" type="submit" fullWidth disabled={isSubmitting} form="edit-album-form">{$t('save')}</Button>
</div>
</ModalFooter>
Expand Down
Loading