feat: Global: Notification Alert Revamp #2555

glespinosa · 2024-02-16T03:02:55Z

Will close #2176

Resolved issues:

new revamp design of notifcation
stacking of notification (note the order of the notification is the bottom is always the latest) there's no currently functionality that will reverse that order in notistack
top center positioning of notification
there's a secondary type notification if the other properties will be available

screencast-8-aaeffee09b-7w6v22.manager.dev.zesty.io_8080-2024.02.16-10_04_07.webm

agalin920 · 2024-02-20T02:20:15Z

@zcolah are we really going with stacking snack bars? Just making sure you are aware that strictly goes against the material design guidelines.

agalin920

.

agalin920 · 2024-02-20T02:23:31Z

src/shell/components/global-notifications/GlobalNotifications.js

@@ -33,10 +48,49 @@ export default connect((state) => {
      setDrawerOpen(false);
    });

+    const alertDetails = (kind) => {


This can be converted into a map and moved outside of the component

this function is now moved outside of a component

src/shell/components/global-notifications/GlobalNotifications.js

agalin920 · 2024-02-20T04:05:22Z

src/shell/components/global-notifications/GlobalNotifications.js

+          "& .MuiAlert-icon, .MuiAlert-message, .MuiAlert-action ": {
+            color: "#fff",
+          },
+          "& .MuiAlert-action": {


These changes should be done at the theme level

Can we do these changes one off for this component for now? Just thinking if this will affect on other part of website that are using Alert Component..

That is the expected process unless Zosh specifically asks otherwise

agalin920 · 2024-02-20T04:08:02Z

src/shell/components/global-notifications/GlobalNotifications.js

+          severity,
+          icon,
+          bgColor,
+        });
        // On every render set timeout to hide notices
        const token = setTimeout(() => {


Why is this still needed/how does this interact with the autoHideDuration={8000} prop passed to the snackBarProvider?

It interacts in SnackbarProvider by passing this custom component prop
Components={{ custom: CustomNotification }}
With the use of these props I can get their value in CustomNotification Component which needs those values to display the data in a snackbar and by using the enqueSnackbar it connects to the Provider. Hope that answers the questions.

zcolah · 2024-02-20T05:20:23Z

@zcolah are we really going with stacking snack bars? Just making sure you are aware that strictly goes against the material design guidelines.

@agalin920 You are correct that Material UI guidelines state that the "snackbar" should appear one at a time. They also do state they should be at the bottom center. However showing one toast at a time can lead to a scenario where a user does not immediately see an error message since the previous snackbar is still being shown.

Hence we have instead decided to take the approach the Github Primer system is taking: https://primer.style/components/toast

Also you can see this discussion thread created to discuss this change: https://zesty-io.slack.com/archives/C03TKNN7XRN/p1704812236712669

agalin920 · 2024-02-21T05:35:37Z

src/shell/components/global-notifications/GlobalNotifications.js

+              maxWidth: "516px",
+              whiteSpace: props.severity === "success" ? "nowrap" : "normal",
+              overflow: "hidden",
+              textOverflow: "ellipsis",


Typography has a noWrap prop that does this

done using noWrap prop on typography

zcolah · 2024-02-21T17:47:08Z

Note: This project cannot deploy until we improve the microcopy for the alerts.
For us to improve the microcopy we need to complete the following issue first: #2416

So please mark this as blocked.

agalin920 · 2024-02-28T17:42:12Z

Note: This project cannot deploy until we improve the microcopy for the alerts. For us to improve the microcopy we need to complete the following issue first: #2416

So please mark this as blocked.

I would NOT want this to be in the same body of work just fyi. I also don't see how this blocks this current ticket.

agalin920 · 2024-02-28T17:43:36Z

@zcolah are we really going with stacking snack bars? Just making sure you are aware that strictly goes against the material design guidelines.

@agalin920 You are correct that Material UI guidelines state that the "snackbar" should appear one at a time. They also do state they should be at the bottom center. However showing one toast at a time can lead to a scenario where a user does not immediately see an error message since the previous snackbar is still being shown.

Hence we have instead decided to take the approach the Github Primer system is taking: https://primer.style/components/toast

Also you can see this discussion thread created to discuss this change: https://zesty-io.slack.com/archives/C03TKNN7XRN/p1704812236712669

I disagree with us taking the approach of a completely different design system that we have not been abiding to. If you have specific product needs for it thats fine but I don't expect the justification to be in another design system

zcolah · 2024-02-28T18:59:46Z

1. The sequence of the projects that were planned were are as follows: a. We get an inventory (Google Sheet) of all the existing notifications from the dev in charge b. We update the notification messages in the system (done by me once part a is done) c. After which we would update the UI. 2. We made a team decision to ship this sequence of projects together as a rollup and expected the sequence to be followed. That being said doing part C first is still fine but we need to wait on the decisions. 3. Our design system is primarily based on Material UI. However we have taken inspiration for a variety of different design systems in the past as well when designing components. For example, our title bars, app sidebars, tabs, text inputs, amongst others have been heavily influenced from other design systems due to visual or usability needs. When designing the notification system we looked at a variety of different systems and methods of communicating notifications. We analyzed multiple design systems and UIs as well for it. And we also made a discussion post to receive feedback on the same in product. Only after our due diligence and a hand off review did we place this ticket in the Design Ready pile of the roadmap.

…

On Wed, Feb 28, 2024, 11:13 PM Andres Galindo ***@***.***> wrote: @zcolah <https://github.com/zcolah> are we really going with stacking snack bars? Just making sure you are aware that strictly goes against the material design guidelines. @agalin920 <https://github.com/agalin920> You are correct that Material UI guidelines state that the "snackbar" should appear one at a time. They also do state they should be at the bottom center. However showing one toast at a time can lead to a scenario where a user does not immediately see an error message since the previous snackbar is still being shown. Hence we have instead decided to take the approach the Github Primer system is taking: https://primer.style/components/toast Also you can see this discussion thread created to discuss this change: https://zesty-io.slack.com/archives/C03TKNN7XRN/p1704812236712669 I disagree with us taking the approach of a completely different design system that we have not been abiding to — Reply to this email directly, view it on GitHub <#2555 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFMT477PMML2MXT27E6RHBTYV5UFJAVCNFSM6AAAAABDLKOK42VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRZGUYTKNJVHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

zcolah · 2024-05-06T22:53:12Z

What is the status of this coming into Beta?

shrunyan · 2024-05-06T23:40:25Z

package.json

@@ -76,6 +76,7 @@
    "lodash": "^4.17.20",
    "moment-timezone": "^0.5.33",
    "monaco-editor": "^0.25.2",
+    "notistack": "^3.0.1",


How much file size does this add to our bundle?

24.3 kB minified, 8.4kb minified + gzipped

agalin920

Can we remove all the commented code?

agalin920 · 2024-05-08T15:59:59Z

src/apps/content-editor/src/app/views/ItemEdit/Meta/ItemSettings/settings/ItemParent.js

                      })
                    );
                  }
                } else {
                  props.dispatch(
                    notify({
                      kind: "warn",
-                      message: `API failed to return data ${res.status}`,
+                      message: `API failed to return data.Try Again.`,


Missing space after period

done fixing.

agalin920 · 2024-05-08T16:00:56Z

src/apps/content-editor/src/app/views/ItemEdit/Meta/ItemSettings/settings/ItemParent.js

                    })
                  );
                }
              } else {
                props.dispatch(
                  notify({
                    kind: "warn",
-                    message: `API failed to return response when searching for ${parentZUID}`,
+                    heading: `"Cannot Save: [${props.metaTitle}]`,


Why does this message hold the metaTitle in brackets but others don't. Shouldn't we be consistent?

My bad on that one, i didn't see that it has [ ]. I removed that now also

agalin920 · 2024-05-08T16:04:44Z

src/shell/components/global-notifications/GlobalNotifications.js

+          "& .MuiAlert-icon, .MuiAlert-message, .MuiAlert-action ": {
+            color: "#fff",
+          },
+          "& .MuiAlert-action": {


That is the expected process unless Zosh specifically asks otherwise

agalin920 · 2024-05-08T16:07:18Z

src/shell/components/global-notifications/GlobalNotifications.js

+          },
+          padding: "8px 12px",
+          borderRadius: "8px",
+          bgcolor: props.bgColor,


The bgColor should be applied by the severity not custom applied. Is there a reason we are doing this?

I removed now also the bgColor i moved it in the theme level. but the reason why i use a bgcolor because the background that variant filled, severity success and other severities are not using the colors that i wanted.

agalin920 · 2024-05-08T16:11:20Z

src/shell/components/global-notifications/GlobalNotifications.js

+            pl: 2,
+            mr: 0,
+          },
+          padding: "8px 12px",


As mentioned lots of custom css on this. We should move this all to the theme

Move the styling now in theme for your review also @agalin920
zesty-io/material#99

Thank you

src/apps/content-editor/src/app/views/ItemEdit/ItemEdit.js

finnar-bin · 2024-05-09T01:26:44Z

src/shell/components/global-notifications/GlobalNotifications.js

+                      key={i}
+                      className={cx(styles.Notification, styles.bodyText)}
+                    >
+                      <p className={styles.Message}>


Wouldn't it be better to convert the elements in this component to MUI components instead?

If I update this to MUI Components it will update also the designs cause its using old design. If that won't be an issue I can proceed to update it?

- remove [ ] characters in between of text - remove setting bgColor, it will now depend on severity base on the theme change

agalin920 · 2024-05-16T22:58:05Z

src/shell/components/global-notifications/GlobalNotifications.js

+  useSnackbar,
+} from "notistack";
+
+const alertDetails = (kind) => {


A static object seems like a better solution for this use case

const alertDetails = {
'warn': {
icon: ...
severity: ...
}
}

@agalin920 done updating to it to static object

agalin920 · 2024-05-17T15:45:33Z

src/apps/schema/src/app/components/CreateModelDialogue.tsx

+        message = "Please Add Display Name";
+        heading = "Cannot Create Model";
+        // @ts-ignore
+      } else if (error?.data?.error.includes("name is already in use")) {


Not a fan of this. Would prefer BE messaging changes. But approving

glespinosa · 2024-05-22T21:55:30Z

@agalin920 for your review regarding the fix on test suite. thank you

Will close #2176 Resolved issues: - [x] new revamp design of notifcation - [x] stacking of notification (note the order of the notification is the bottom is always the latest) there's no currently functionality that will reverse that order in notistack - [x] top center positioning of notification - [x] there's a secondary type notification if the other properties will be available ![image](https://github.com/zesty-io/manager-ui/assets/44116036/85b12c3f-c130-4e86-bac3-976c1e054ecd) ![image](https://github.com/zesty-io/manager-ui/assets/44116036/bf0b8c65-40b1-4302-8072-a36de3c03988) [screencast-8-aaeffee09b-7w6v22.manager.dev.zesty.io_8080-2024.02.16-10_04_07.webm](https://github.com/zesty-io/manager-ui/assets/44116036/68ce2563-a155-4749-bc03-fcaa93a8331d) --------- Co-authored-by: Stuart Runyan <[email protected]>

revamp alert notification

6cc783c

glespinosa self-assigned this Feb 16, 2024

glespinosa changed the title ~~revamp alert notification~~ feat: Global: Notification Alert Revamp Feb 16, 2024

glespinosa requested review from agalin920 and finnar-bin February 19, 2024 23:35

glespinosa marked this pull request as ready for review February 19, 2024 23:35

agalin920 requested changes Feb 20, 2024

View reviewed changes

agalin920 reviewed Feb 21, 2024

View reviewed changes

glespinosa added the blocked label Feb 22, 2024

glespinosa marked this pull request as draft February 22, 2024 04:18

shrunyan changed the base branch from master to dev March 25, 2024 19:20

shrunyan and others added 7 commits April 4, 2024 12:46

Merge branch 'dev' into feat/revamp-alert-notification

000611e

Merge branch 'dev' into feat/revamp-alert-notification

6798312

Merge branch 'dev' into feat/revamp-alert-notification

06bbda6

Content Update Global Alert

20beb6d

Schema: Update Global Alert

149f529

Merge branch 'dev' into feat/revamp-alert-notification

2c5a413

Merge branch 'dev' into feat/revamp-alert-notification

4d5cd58

glespinosa marked this pull request as ready for review April 30, 2024 06:13

glespinosa linked an issue May 6, 2024 that may be closed by this pull request

Content: Improved Microcopy for Notifications #2416

Closed

shrunyan enabled auto-merge (squash) May 6, 2024 23:39

glespinosa removed the blocked label May 6, 2024

glespinosa added the ready PR is complete and ready for deployment label May 6, 2024

shrunyan reviewed May 6, 2024

View reviewed changes

glespinosa requested review from agalin920 and shrunyan May 7, 2024 10:42

Merge branch 'dev' into feat/revamp-alert-notification

2cfd2ba

agalin920 requested changes May 8, 2024

View reviewed changes

finnar-bin reviewed May 9, 2024

View reviewed changes

- removes commented messages

0071e3f

- remove [ ] characters in between of text - remove setting bgColor, it will now depend on severity base on the theme change

glespinosa requested review from agalin920 and finnar-bin May 9, 2024 22:13

agalin920 requested changes May 16, 2024

View reviewed changes

convert function to static object

49e0639

agalin920 reviewed May 17, 2024

View reviewed changes

agalin920 previously approved these changes May 17, 2024

View reviewed changes

glespinosa and others added 2 commits May 21, 2024 06:30

Merge branch 'dev' into feat/revamp-alert-notification

afcc027

actions.spec.js test suite update

ffb7842

glespinosa dismissed agalin920’s stale review via ffb7842 May 22, 2024 09:19

update content.spec.js toast contains

41b2836

glespinosa requested a review from agalin920 May 22, 2024 21:55

shrunyan and others added 2 commits May 23, 2024 12:57

Merge branch 'dev' into feat/revamp-alert-notification

3ff19c2

Merge branch 'dev' into feat/revamp-alert-notification

1ac385b

agalin920 approved these changes May 24, 2024

View reviewed changes

finnar-bin approved these changes May 27, 2024

View reviewed changes

shrunyan added 2 commits May 30, 2024 16:09

Merge branch 'dev' into feat/revamp-alert-notification

b98c3a8

Merge branch 'dev' into feat/revamp-alert-notification

af7b250

shrunyan merged commit ba9f951 into dev May 30, 2024
1 check passed

shrunyan deleted the feat/revamp-alert-notification branch May 30, 2024 23:44

feat: Global: Notification Alert Revamp #2555

feat: Global: Notification Alert Revamp #2555

Uh oh!

Conversation

glespinosa commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agalin920 commented Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agalin920 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zcolah commented Feb 20, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zcolah commented Feb 21, 2024

Uh oh!

agalin920 commented Feb 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agalin920 commented Feb 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zcolah commented Feb 28, 2024 via email

Uh oh!

zcolah commented May 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agalin920 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glespinosa May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

finnar-bin May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glespinosa commented Feb 16, 2024 •

edited

Loading

agalin920 commented Feb 20, 2024 •

edited

Loading

agalin920 commented Feb 28, 2024 •

edited

Loading

agalin920 commented Feb 28, 2024 •

edited

Loading

glespinosa May 9, 2024 •

edited

Loading

finnar-bin May 9, 2024 •

edited

Loading

agalin920 May 16, 2024 •

edited

Loading