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

192137 remove banner #622

Merged
merged 8 commits into from
Apr 23, 2024
Merged

192137 remove banner #622

merged 8 commits into from
Apr 23, 2024

Conversation

krischarbonneau
Copy link
Contributor

@krischarbonneau krischarbonneau commented Apr 22, 2024

ADO-192137

Fixed AB#192137

Description of proposed changes:

This PR removes the PhaseBanner component as it is no longer being used. This PR also gets rid of the old MultiModal component as we are only using a single modal for timeout purposes now and as such has been replaced by the new IdleTimeout component. This PR also removes the 3 unused mappers and all references to them as they are no longer needed. I have chosen to use local translations for the timeout modal for now as AEM was giving me headaches but we can always explore switching back to using AEM at a later date for this.

What to test for/How to test

  1. Pull in branch
  2. Set the values for timeout and promptBeforeIdle in the IdleTimeout component to something really small (setting the last number to something like 15 as an example)
  3. Type npm run dev
  4. Navigate to the dashboard and don't move your mouse until the modal pops up. Ensure it works as expected
  5. Ensure that the banner is not showing on any of the pages

Additional Notes

The Layout unit tests are still being skipped due to issues with how the AA prefix is passed down. This will need to be looked into.

@krischarbonneau krischarbonneau requested a review from a team as a code owner April 22, 2024 19:18
Copy link

components/IdleTimeout.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@shewood shewood left a comment

Choose a reason for hiding this comment

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

Looks good but a couple of changes for AA needed.

Copy link
Collaborator

@shewood shewood left a comment

Choose a reason for hiding this comment

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

Nice.

@krischarbonneau krischarbonneau merged commit 5c4b448 into dev Apr 23, 2024
8 checks passed
@krischarbonneau krischarbonneau deleted the 192137-remove-banner branch April 23, 2024 15:58
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