Skip to content

Conversation

aasandei-vsp
Copy link
Contributor

Copy link

codecov bot commented Sep 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.22%. Comparing base (bd75e6a) to head (e8fd295).
⚠️ Report is 31 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #707      +/-   ##
==========================================
+ Coverage   45.20%   45.22%   +0.02%     
==========================================
  Files         370      367       -3     
  Lines       11256    11234      -22     
  Branches     1855     1850       -5     
==========================================
- Hits         5088     5081       -7     
+ Misses       6002     5985      -17     
- Partials      166      168       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aasandei-vsp aasandei-vsp marked this pull request as draft September 4, 2025 12:15
@aasandei-vsp aasandei-vsp marked this pull request as ready for review September 5, 2025 15:57
Copy link
Member

@cecilia-donnelly cecilia-donnelly left a comment

Choose a reason for hiding this comment

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

Thank you @aasandei-vsp ! Can you make the text bigger? I know it's hard to match without an official design, but the fonts for the date and explanatory text should be closer to the title size - if you look at the example image, the text inside the circle image is about the same size as the list items and general explanation. On my screen, at least, those look smaller in this PR.

I did try different responsive screen sizes and at a certain point the explanatory text overlaps the image, but I think that's probably fine since it only appears to affect a specific (odd) screen size.

I love the action button - it looks beautiful!

@aasandei-vsp
Copy link
Contributor Author

Thank you @aasandei-vsp ! Can you make the text bigger? I know it's hard to match without an official design, but the fonts for the date and explanatory text should be closer to the title size - if you look at the example image, the text inside the circle image is about the same size as the list items and general explanation. On my screen, at least, those look smaller in this PR.

I did try different responsive screen sizes and at a certain point the explanatory text overlaps the image, but I think that's probably fine since it only appears to affect a specific (odd) screen size.

I love the action button - it looks beautiful!

I have made the text bigger, it's an easy change, so if you'd like me to tweak it even more, that can be done.

Indeed, at some point the text overlaps the image, I also thought that's an odd screen size so I didn't put much effort into it, but I could add some media queries for that specific case if needed.

I'm glad you like the choice for the link :D

@cecilia-donnelly cecilia-donnelly self-requested a review September 8, 2025 15:40
Copy link
Member

@cecilia-donnelly cecilia-donnelly left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you @aasandei-vsp.

I mentioned this in our meeting, but to put it in writing: we prefer not to have merge commits in the history, so please rebase this on main and remove those. Thank you!

@aasandei-vsp aasandei-vsp force-pushed the PER-10283-change-login-screen-graphic-for-legacy-lab branch 4 times, most recently from 1705411 to c460d99 Compare September 9, 2025 13:48
@omnignorant
Copy link
Member

omnignorant commented Sep 17, 2025

Great work @aasandei-vsp. I immediately encountered the text overlap issue Andrei noted in his review, so not sure if that size is as odd as he suggests. Would it be possible to set a min screen width so that any downsizing below that width doesn't scale and requires the user to horizontal scroll in order to see everything? Or maybe we drop the graphic and only keep the text on a blue background at the intermediate width?

If we can't prevent this, then let's just use a solid blue background and no graphic at all window sizes.

Screenshot 2025-09-17 at 12 07 19 PM

Copy link
Member

@omnignorant omnignorant left a comment

Choose a reason for hiding this comment

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

see prior comments

@aasandei-vsp aasandei-vsp force-pushed the PER-10283-change-login-screen-graphic-for-legacy-lab branch from c460d99 to 6767b63 Compare September 18, 2025 12:13
@aasandei-vsp
Copy link
Contributor Author

@omnignorant I have made some changes in order to remove the background image when the text and the logo overlap. Do you think you could have another look and let me know if that's ok or if you would prefer a different approach?

@aasandei-vsp
Copy link
Contributor Author

We will not advertise this now, but do it in March.

@slifty slifty deleted the PER-10283-change-login-screen-graphic-for-legacy-lab branch October 9, 2025 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants