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

Remove the old welcome dialog and invitation dialog components #493

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

crisnicandrei
Copy link
Contributor

I have removed the old welcome dialogs because with the new onboarding they are not needed anymore.

Steps to test:

  1. Create a new account (with or without invitation)
  2. Go through the onboarding process
  3. When done, it should redirect you to the private workspace, without any dialog popping up

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.26%. Comparing base (8b882a6) to head (f66ed30).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #493      +/-   ##
==========================================
+ Coverage   43.25%   43.26%   +0.01%     
==========================================
  Files         363      363              
  Lines       11102    11104       +2     
  Branches     1809     1810       +1     
==========================================
+ Hits         4802     4804       +2     
  Misses       6139     6139              
  Partials      161      161              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@meisekimiu meisekimiu left a comment

Choose a reason for hiding this comment

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

We do not want to actually delete these components at this stage of Glam development, but instead remove any links and references to them in the Glam onboarding flow while keeping them intact for the legacy flow that is still in use. That way, we can merge this as soon as possible without disrupting the current flow.

To be clear, the current plan to minimize disruption of the current flow is to:

  1. Build up the Glam onboarding flow in isolation from the other one
  2. Continue making incremental improvements that are frequently deployed to all environments, even though they're not visible <- We are here
  3. Integrate feature flags into the onboarding flow so developers and staff can test it in lower environments (this should have come earlier but we did not have server-side feature flag support when this work started)
  4. Once the quality of the onboarding flow is satisfactory, we enable the new onboarding feature flag in production, releasing the feature for our actual users.
  5. Once the new flow has been in use for a while, we can remove the feature flags and delete the unused legacy onboarding flow.

So while we eventually want to delete these components, at this stage in development we can't do that without breaking or at least degrading the old onboarding flow. Until we've fully released the new onboarding, we should be asking ourselves with each pull request "could this go to production right now without any noticeable changes?"

Strategies like this encourage smaller pull requests that can be merged more frequently so we don't end up having PRs stuck open for weeks or even months.

@crisnicandrei crisnicandrei force-pushed the PER-9766-remove-old-congrats-screen branch 2 times, most recently from de89325 to 9185b50 Compare November 26, 2024 17:00
@crisnicandrei
Copy link
Contributor Author

@meisekimiu Thank you for your explanation! I have reverted the changes

@crisnicandrei crisnicandrei requested a review from k8lyn6 November 28, 2024 13:04
@k8lyn6 k8lyn6 requested a review from yeslikesolo December 2, 2024 14:13
@k8lyn6
Copy link

k8lyn6 commented Dec 2, 2024

@yeslikesolo For testing, go through Glam onboarding (using multiple ways, such as archive member, etc) and make sure the final pop up over the workspace doesn't show up. Then, test non-Glam and make sure it does pop up.

@crisnicandrei crisnicandrei force-pushed the PER-9766-remove-old-congrats-screen branch from 3866a9d to 90b2df9 Compare December 12, 2024 08:47
This pr reverts the removal of the dialog component from the onboarding component and only redirects the user after completing the process
@meisekimiu meisekimiu force-pushed the PER-9766-remove-old-congrats-screen branch from 90b2df9 to f66ed30 Compare December 12, 2024 22:14
@meisekimiu meisekimiu merged commit 0a3ef11 into main Dec 12, 2024
2 checks passed
@meisekimiu meisekimiu deleted the PER-9766-remove-old-congrats-screen branch December 12, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants