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

Update trial messages in both vscode extension and trial web page #1461

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

mabashian
Copy link
Member

Jira Issue: https://issues.redhat.com/browse/AAP-36175

Description

This updates the messages that we present to the user when a trial will expire in <= 90 days as well as when a trial is expired.

Testing

I tested out all of the various scenarios by manipulating the expired_at timestamp on the user plan. There are 4 options/scenarios:

  1. A trial hasn't started yet
  2. Expired (date set to something in the past)
  3. About to expire (date set to something within the next 90 days)
  4. Not about to expire (date set to something more than 90 days out). Theoretically this shouldn't be possible but it's a scenario we already cover so I tested it anyway.
Screenshot 2024-12-13 at 1 43 13 PM Screenshot 2024-12-13 at 1 42 46 PM Screenshot 2024-12-13 at 1 42 12 PM Screenshot 2024-12-13 at 1 41 05 PM Screenshot 2024-12-13 at 2 51 27 PM Screenshot 2024-12-13 at 2 50 54 PM

Scenarios tested

See above

Production deployment

  • This code change is ready for production on its own
  • This code change requires the following considerations before going to production:

@manstis
Copy link
Contributor

manstis commented Dec 14, 2024

I'll have a better look Monday @mabashian but it looks like you'll need some unit tests for the logic. It'd be nice too, in my opinion, if the text could wrap on the HTML page. I also notice the three buttons/links are spread far apart. The footer HTML is different on some of the other pages which makes the buttons group better. They're in the same container as the long text at the moment (The other login pages uses a separate container IIRC).

@manstis
Copy link
Contributor

manstis commented Dec 16, 2024

@mabashian Regarding the formatting of the buttons on the footer.

Look at the "Home page" where they are held in their own "Bullseye" container.

This change isn't strictly part of your PR.. but now the text is very long the spacing is more evident.

@mabashian
Copy link
Member Author

@manstis 👍 I'll take a look at that formatting as well

@mabashian mabashian force-pushed the mabashian/AAP-36175-trial-text branch from 340dee6 to 04952e8 Compare December 16, 2024 19:22
@mabashian
Copy link
Member Author

@manstis OK I think I've got the footer spacing issue squared away:

Screenshot 2024-12-16 at 2 45 05 PM

Tell me more about It'd be nice too, in my opinion, if the text could wrap on the HTML page - I think the text is wrapping down to a new line when the screen is more narrow but maybe this isn't working for some browsers?

@mabashian mabashian force-pushed the mabashian/AAP-36175-trial-text branch from 450f5ec to 9482e6e Compare December 16, 2024 19:51
Copy link
Contributor

@manstis manstis left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I think the text is wrapping down to a new line when the screen is more narrow...

Yes it does. The button spacing is much better (and, IMO, more importantly consistent). The (long) length of the single line of text is just my opinion. There's nothing wrong with it.. so ignore me.

@manstis
Copy link
Contributor

manstis commented Dec 17, 2024

This may help with the failing tests: #1466

@mabashian mabashian force-pushed the mabashian/AAP-36175-trial-text branch from 9482e6e to 5731571 Compare December 17, 2024 18:29
@mabashian mabashian force-pushed the mabashian/AAP-36175-trial-text branch from 5731571 to 9ac30ae Compare December 19, 2024 20:35
@mabashian mabashian merged commit 69a10e1 into main Dec 19, 2024
11 checks passed
@mabashian mabashian deleted the mabashian/AAP-36175-trial-text branch December 19, 2024 20:50
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.

2 participants