Skip to content

Conversation

@crisnicandrei
Copy link
Contributor

@k8lyn6 the icons work fine for me

I also need a link for the new underlined text:) Thanks!

@codecov
Copy link

codecov bot commented Nov 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.56%. Comparing base (ec042eb) to head (13ed117).
Report is 77 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #487      +/-   ##
==========================================
- Coverage   42.57%   42.56%   -0.01%     
==========================================
  Files         357      357              
  Lines       10984    10984              
  Branches     1795     1795              
==========================================
- Hits         4676     4675       -1     
  Misses       6149     6149              
- Partials      159      160       +1     

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

@k8lyn6 k8lyn6 requested review from k8lyn6 and yeslikesolo November 7, 2024 14:42
Copy link
Collaborator

@yeslikesolo yeslikesolo left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-11-08 at 2 17 34 PM

  1. Looks like the icons aren't working for me
  2. Clicking on the underlined link "viewing our help articles here." takes me back to the log in screen. I think it should take us to the Knowledge Base instead ? (Idk what the link was before)

@crisnicandrei
Copy link
Contributor Author

@yeslikesolo yes i asked @k8lyn6 for the link because I did not have it

@crisnicandrei
Copy link
Contributor Author

@yeslikesolo can you paste it here please so i can add it?

@k8lyn6
Copy link
Collaborator

k8lyn6 commented Nov 12, 2024

@k8lyn6 k8lyn6 requested a review from yeslikesolo November 12, 2024 14:28
@crisnicandrei
Copy link
Contributor Author

@k8lyn6 @meisekimiu @yeslikesolo updated the url

Copy link
Collaborator

@yeslikesolo yeslikesolo left a comment

Choose a reason for hiding this comment

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

The link works great, thanks @crisnicandrei !

One thing @k8lyn6 and I spoke about was changing the link to open in a new tab. Although it is best practice to open links in the same tab, if clicked on, the link currently lives in an area that disrupts the user flow to complete account creation and move forward in the onboarding process. It also signs them out of the process. Could we change this please? Let me know if you have any questions! Thanks again, Andrei (:

@crisnicandrei
Copy link
Contributor Author

@yeslikesolo pushed the changes

@k8lyn6 k8lyn6 requested a review from yeslikesolo November 19, 2024 14:11
Copy link
Contributor

@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.

(Was waiting for the URL to be resolved before I approved on code review!)

Copy link
Collaborator

@yeslikesolo yeslikesolo left a comment

Choose a reason for hiding this comment

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

Yay, works great. Thanks @crisnicandrei & @meisekimiu !

@meisekimiu meisekimiu force-pushed the PER-9760-upgrade-congrats-screen branch from f92e8e2 to 13ed117 Compare November 19, 2024 20:57
@meisekimiu
Copy link
Contributor

Planning on merging this myself so that I don't get a merge conflict on my own work that touches this component!

@meisekimiu meisekimiu merged commit 3b1f4d5 into main Nov 19, 2024
2 checks passed
@meisekimiu meisekimiu deleted the PER-9760-upgrade-congrats-screen branch November 19, 2024 21:00
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.

5 participants