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

fix(web): small issues everywhere #7207

Merged
merged 6 commits into from
Feb 21, 2024
Merged

fix(web): small issues everywhere #7207

merged 6 commits into from
Feb 21, 2024

Conversation

martabal
Copy link
Member

@martabal martabal commented Feb 19, 2024

This PR:

  • removes start-microservices.sh and start-server.sh usage in favor of start.sh (but still keep them)
  • fix an issue when creating an album where the "Select Photos" re-appear after entering the album name , reported by @alextran1502
  • when creating an album, add a margin-top between the title and the description
  • center the text in the chat in the activity viewer
  • fix the autoGrowHeight for the text-area in the description
  • add a notification when the album / asset description is updated
  • fix for [BUG] ctl+f keyboard shortcut is used to mark a photo as a favorite #5603

Screenshots

Before After
Screenshot from 2024-02-19 19-36-35 Screenshot from 2024-02-19 20-41-11
Screenshot from 2024-02-19 19-36-58 Screenshot from 2024-02-19 19-35-48

@alextran1502
Copy link
Contributor

alextran1502 commented Feb 19, 2024

Have you tested running make prod and see if it works as expected?

@martabal
Copy link
Member Author

Have you tested running make prod and see if it works as expected?

Yes, It works fine

@alextran1502
Copy link
Contributor

alextran1502 commented Feb 19, 2024

Can we use this opportunity to fix the placeholder text color of "Add a title" in dark mode to be a little brighter? I've also found that having a lot of notifications for each action is making the screen busy.

Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

cloudflare-pages bot commented Feb 19, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 52a2b07
Status: ✅  Deploy successful!
Preview URL: https://ac62e41b.immich.pages.dev
Branch Preview URL: https://fix-multiple.immich.pages.dev

View logs

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

LGTM

@alextran1502
Copy link
Contributor

@jrasm91 This change doesn't affect the user, as I understand, right?

@jrasm91
Copy link
Contributor

jrasm91 commented Feb 20, 2024

@jrasm91 This change doesn't affect the user, as I understand, right?

That is incorrect. Existing installations will have docker compose files with the start command referencing the bash script start-*.sh. Those files will no longer exist in the docker image and the containers will fail to come up. It is a breaking change because users are required to update their compose files and change the start commands. This doesn't only apply to docker compose projects, but any references to those custom start commands (of which I believe there are many) will become invalid if we delete the files.

@alextran1502
Copy link
Contributor

@jrasm91 The change here is for the docker-compose.prod.yml not docker-compose.yml though

@martabal
Copy link
Member Author

Yes, I don’t think it’s a breaking change

@etnoy
Copy link
Contributor

etnoy commented Feb 20, 2024

Yes, I don’t think it’s a breaking change

It is a breaking change since many users still use the old syntax and runs the .sh file directly from their docker compose. Those users need to update their compose file.

Nevertheless, I think we should merge this before 1.95 and include a breaking change notice

@jrasm91
Copy link
Contributor

jrasm91 commented Feb 20, 2024

@jrasm91 The change here is for the docker-compose.prod.yml not docker-compose.yml though

It doesn't matter. The breaking change is the fact that the start-microservices.sh file is being deleted.

@martabal martabal marked this pull request as draft February 20, 2024 15:54
@martabal martabal marked this pull request as ready for review February 20, 2024 17:36
@@ -570,6 +555,8 @@
};
</script>

<svelte:window on:keydown={handleKeypress} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

web/src/routes/(user)/albums/[albumId]/+page.svelte Outdated Show resolved Hide resolved
@jrasm91 jrasm91 merged commit 8f57bfb into main Feb 21, 2024
27 checks passed
@jrasm91 jrasm91 deleted the fix/multiple branch February 21, 2024 13:29
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

5 participants