feat(styles): notification in dialog

[platon-rov]

Related Issue

Refers to SAP/fundamental-ngx#8525

Description

Notifications in dialog update.

Screenshots

After:

[netlify]

✅ Deploy Preview for fundamental-styles ready!

Name	Link
🔨 Latest commit	ee17515
🔍 Latest deploy log	https://app.netlify.com/sites/fundamental-styles/deploys/6332c259b3e95c0008c887d0
😎 Deploy Preview	https://deploy-preview-3818--fundamental-styles.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[InnaAtanasova]

• There's no way to dismiss the dialog

• on mobile the dialog should take the whole screen. Here are the guidelines: https://experience.sap.com/internal/fiori-design-web/dialog/#size-s-smartphone

Also check the UX guidelines: https://experience.sap.com/internal/fiori-design-web/notification-center/

[platon-rov]

• There's no way to dismiss the dialog

Fixed.

• On mobile the dialog should take the whole screen.
  Here are the guidelines: https://experience.sap.com/internal/fiori-design-web/dialog/#size-s-smartphone
  Also check the UX guidelines: https://experience.sap.com/internal/fiori-design-web/notification-center/

There is nothing about the mobile dialog, it's just about the notifications in the dialog. If needed, the dialog may be additionally configured.

[InnaAtanasova]

You should probably use the compact button and align it with the other dismiss buttons (that belong to the notifications) as this will be immediately reported as a bug by the designers.

[InnaAtanasova]

• There's no way to dismiss the dialog

Fixed.

• On mobile the dialog should take the whole screen.
  Here are the guidelines: https://experience.sap.com/internal/fiori-design-web/dialog/#size-s-smartphone
  Also check the UX guidelines: https://experience.sap.com/internal/fiori-design-web/notification-center/

There is nothing about the mobile dialog, it's just about the notifications in the dialog. If needed, the dialog may be additionally configured.

I provided these links as it specifies that the actions are in the footer:
"On smartphones, a dialog can have one or two actions, which are located in the footer and right-aligned."
This would mean that dismissing the notification dialog should be done by a Cancel button in the footer, not in the header (in my opinion)

[platon-rov]

You should probably use the compact button and align it with the other dismiss buttons (that belong to the notifications) as this will be immediately reported as a bug by the designers.

Not a bug actually. Dialog has 1rem padding on the sides and notification has 0.5rem on the right side.

I provided these links as it specifies that the actions are in the footer:

Fixed.

[mikerodonnell89]

Dialog can be too wide for the notifications, depending on screen width:

[mikerodonnell89]

looks good! I do agree with Inna that the margins look strange but if that's the design, it is out of our hands. Definitely looks a little better with the cancel button in the footer rather than the X bottom in the header.

[InnaAtanasova]

Should it look like this in mobile?

[platon-rov]

Should it look like this in mobile?

Nothing about notification itself, it comes from the dialog.

[InnaAtanasova]

Should it look like this in mobile?

Nothing about notification itself, it comes from the dialog.

it doesn't matter :) We shouldn't merge something that is broken or doesn't look ok. If it's coming from the dialog, either open a new issue that should be fixed asap and link it here, or fix it in a separate PR and link it here. But we can't merge this PR and just ignore the fact that in mobile the notifications look broken. @droshev what is your opinion?

[platon-rov]

it doesn't matter :) We shouldn't merge something that is broken or doesn't look ok. If it's coming from the dialog, either open a new issue that should be fixed asap and link it here, or fix it in a separate PR and link it here. But we can't merge this PR and just ignore the fact that in mobile the notifications look broken. @droshev what is your opinion?

Opened a pr that fixes this issue

#3882