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): onboarding #6066

Merged
merged 37 commits into from
Jan 4, 2024
Merged

feat(web): onboarding #6066

merged 37 commits into from
Jan 4, 2024

Conversation

alextran1502
Copy link
Contributor

@alextran1502 alextran1502 commented Dec 30, 2023

This PR adds onboarding steps for new instance to configure

  • Theme
  • Storage template
onboarding.mp4

Copy link

cloudflare-pages bot commented Dec 30, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 933a86f
Status: ✅  Deploy successful!
Preview URL: https://2925aa98.immich.pages.dev
Branch Preview URL: https://dev-onboarding.immich.pages.dev

View logs

@alextran1502 alextran1502 marked this pull request as ready for review January 1, 2024 06:10
bo0tzz
bo0tzz previously requested changes Jan 1, 2024
Copy link
Member

@bo0tzz bo0tzz left a comment

Choose a reason for hiding this comment

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

Awesome stuff!

The storage template step currently has both a save button (from the template settings panel) and a done button (from the onboarding flow). I think it would be better if there is only one button that both saves the settings and completes/continues the onboarding.

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.

Great work!
Imo the theme buttons can be a little smaller. They're currently huuuuge

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.

All the server changes seem to be adding the showOnboarding property to the user's table. I think it might make more sense for this to be a setting per instance instead of per user.

One option would be to just revert all the server-side changes, and then add isOnboarded next to isInitialized in the ServerConfigDto, which is returned from /api/server-info/config. When true, show the onboarding screen. Save this setting in the database via the ISystemMetadataRepository.

@alextran1502 alextran1502 enabled auto-merge (squash) January 4, 2024 03:22
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 (besides the fact there are no tests)

@jrasm91 jrasm91 dismissed bo0tzz’s stale review January 4, 2024 05:24

the concern has been addressed

@alextran1502 alextran1502 merged commit 18f59f7 into main Jan 4, 2024
20 checks passed
@alextran1502 alextran1502 deleted the dev/onboarding branch January 4, 2024 05:28
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

martabal pushed a commit that referenced this pull request Jan 9, 2024
* feat(web): onboarding

* feat: openapi

* feat: modulization

* feat: page advancing

* Animation

* Add storage templaete settings

* sql

* more style

* Theme

* information and styling

* hide/show table

* Styling

* Update user property

* fix test

* fix test:

* fix e2e

* test

* Update web/src/lib/components/onboarding-page/onboarding-hello.svelte

Co-authored-by: bo0tzz <git@bo0tzz.me>

* naming

* use System Metadata

* better return type

* onboarding using server metadata

* revert previous changes in user entity

* sql

* test web

* fix test server

* server/web test

* more test

* consolidate color theme change logic

* consolidate save button to storage template

* merge main

* fix web

---------

Co-authored-by: bo0tzz <git@bo0tzz.me>
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

4 participants