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

SLVUU-69: Display toast on layout load/save #115

Closed
wants to merge 5 commits into from

Conversation

pling-scottlogic
Copy link

Description

Display toast notifications for all relevant scenarios pertaining to layout management.

Change List

  • Wrap the ShellWithNewThemeAndLayoutManagement Showcase example with NotificationsProvider
  • Call notify from useNotifications in the following instances:
    • Loading metadata returns an error
    • Loading the application layout returns an error
    • A layout is saved successfully
    • Saving a layout returns an error
    • An attempt is made to save an undefined layout

Testing

Manually tested on the ShellWithNewThemeAndLayoutManagement Showcase example by configuring the app to use remote layout management:

  • Loading the example without first running the layout server results in an error toast being displayed
  • Saving a layout (with the layout server running) results in a success toast being displayed

Evidence

Success toast displayed after saving a layout:

SLVUU69-toast-success

Error toast displayed after shutting down the layout server and refreshing the app:

SLVUU69-toast-error

<ShellWithNewTheme />
</LayoutManagementProvider>
<NotificationsProvider>
<LayoutManagementProvider>

Choose a reason for hiding this comment

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

can we do this for sample app as well? (vuu-ui/sample-apps/app-vuu-example/index.tsx)

Copy link
Author

Choose a reason for hiding this comment

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

Copy link

@cfisher-scottlogic cfisher-scottlogic left a comment

Choose a reason for hiding this comment

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

Just a couple of comments

Copy link

@cfisher-scottlogic cfisher-scottlogic left a comment

Choose a reason for hiding this comment

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

Should we add a case for when a user's layout fails to load? It's obvious that it's loaded successfully because you can see it. Is it obvious that it hasn't loaded successfully if you don't see it? Probably...

Copy link

@vferraro-scottlogic vferraro-scottlogic left a comment

Choose a reason for hiding this comment

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

LGTM

@pling-scottlogic
Copy link
Author

Should we add a case for when a user's layout fails to load? It's obvious that it's loaded successfully because you can see it. Is it obvious that it hasn't loaded successfully if you don't see it? Probably...

Done: 7210e13

@pling-scottlogic pling-scottlogic changed the base branch from VUU333-layout-server to main November 22, 2023 17:11
@pling-scottlogic
Copy link
Author

Merged into Finos main under finos#1004

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.

Notification Toast for Layout Manager
3 participants