(feat) O3-2546: Add the ability to use actions in the existing toast #822
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -716 kB (-21%) 🎉 Total Size: 2.75 MB
ℹ️ View Unchanged
|
|
@ibacher @denniskigen this PR is ready for review, please take a look. |
ibacher
reviewed
Nov 30, 2023
There was a problem hiding this comment.
Thanks @hadijahkyampeire! A few comments and questions here.
packages/apps/esm-implementer-tools-app/src/implementer-tools.component.tsx
Outdated
Show resolved
Hide resolved
packages/framework/esm-react-utils/__mocks__/openmrs-esm-styleguide.mock.tsx
Outdated
Show resolved
Hide resolved
packages/framework/esm-styleguide/src/actionable-toasts/actionable-toast.component.tsx
Outdated
Show resolved
Hide resolved
packages/framework/esm-styleguide/src/actionable-toasts/actionable-toast.component.tsx
Outdated
Show resolved
Hide resolved
| @@ -44,6 +44,7 @@ | |||
| <body> | |||
| <%= htmlWebpackPlugin.tags.bodyTags %> | |||
| <div class="omrs-toasts-container"></div> | |||
| <div class="omrs-actionable-toasts-container"></div> | |||
There was a problem hiding this comment.
Have we looked into how things look when there are both toasts and actionable toasts?
There was a problem hiding this comment.
Actually not yet but I am going to test and see.
There was a problem hiding this comment.
This shows they stuck on top of each other but since we are going to replace the toasts with the snackbar and then get rid of the Toast component, maybe we do not need to worry about that. cc @denniskigen @ibacher
There was a problem hiding this comment.
@ibacher would you propose we just modify the existing toast to accept actions or let new actionable toast use the same container for toast.
There was a problem hiding this comment.
Missed this since it got marked as resolved. I'd favour having all toasts in a single container.
hadijahkyampeire
changed the title
hadijahkyampeire
force-pushed
the
hk/ft/actionable-toast/O3-2546
branch
from
0d434f7
to
3fd2ed6
Compare
|
@ibacher @denniskigen I have updated this PR to add the implementation to allow the use of an action, so it won't break anything as far as the existing toast works and for the parts where we want to add an action it can also allow. Please see the attached recording and review the changes. |
ibacher
requested changes
Dec 15, 2023
There was a problem hiding this comment.
Couple of things. Last question: are we sure that all actionable notifications should always remain until the user does something with them? I'd favour a more flexible system to allow the producer of the notification to decide. (For example, email applications now tend to offer an "undo send" feature, which displays an action you can take up until a certain timeout, after which the action goes away because the email has been sent).
packages/framework/esm-styleguide/src/toasts/toast.component.tsx
Outdated
Show resolved
Hide resolved
|
@ibacher I have implemented the changes, please take another look. |
hadijahkyampeire
force-pushed
the
hk/ft/actionable-toast/O3-2546
branch
from
02d9bb3
to
04f9aab
Compare
|
Is this good to go, @ibacher? |
ibacher
approved these changes
Jan 8, 2024
|
Yes, I think so. |
|
Thanks @ibacher |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
For changes to apps
If applicable
Summary
Screenshots
Recording
toast-actions.mov
Related Issue