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.

Sign up for GitHub

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

Jump to bottom

fix: Remove MindLogger update version banner (M2-8082) #1970

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix: Remove MindLogger update version banner (M2-8082) #1970

wants to merge 2 commits into from
+2 −22

Conversation

AlejandroCoronadoN
Copy link
Contributor

@AlejandroCoronadoN AlejandroCoronadoN commented Oct 31, 2024 •
edited by farmerpaul
Loading

📝 Description

As an Admin I want the MindLogger version Info banner to be removed to avoid confusion for new Admin users.

🔗 Jira Ticket M2-8082

Changes include:

  • [Removed addBanner action when session status changes avoiding rendering VersionWarningBanner]
  • [Updated tests for VersionWarningBanner and only testing for removing all banners action when sesion status changes]

📸 Screenshots

Before (Optional) After
image image

🪤 Peer Testing

  1. Start your admin app
  2. Login with your user
  3. "Update version Banner" () shouldn't be displayed at the top of the screen

✏️ Notes

  • After tlaking with Cari, Marty and Paul, we decided that the best approach was to avoid rendering the Banner component instead of entirely delete it because we might rehuse it in the future.

@AlejandroCoronadoN
Removed VersionWarningBanner when session becomes valid. This change …
4362500 
…avoids rendering the VersionWarningBanner but keeping the Banner inside components for future references or use
@AlejandroCoronadoN
Deleted comment that referenced VersionWarningBanner
4186fa1
@aws-amplify-us-east-1 AWS Amplify (us-east-1)
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1970.d19gtpld8yi51u.amplifyapp.com

farmerpaul
farmerpaul approved these changes Nov 4, 2024
View reviewed changes
Copy link
Contributor

@farmerpaul farmerpaul left a comment

Choose a reason for hiding this comment

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

Tests pass, and code and assumptions are all good @AlejandroCoronadoN, nice work! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@farmerpaul farmerpaul farmerpaul approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AlejandroCoronadoN @farmerpaul