fix: make error box dismissable #44
Conversation
There was a problem hiding this comment.
This does not make the error box dismissable, instead it hides the error after 5 seconds.
This is a distinctly different solution, and has usability issues. If the user does not pay attention, or is on another screen while the error happens, they might not even see the error message.
This makes it also very hard for users to share error messages (because they will immediately disappear).
I have updated the issue description to reflect that user interaction should be needed to dismiss the error box.
|
How would you like to get it dismissed? do you want a cross button to dismiss it? what user action should make it disappear? |
|
A |
|
I am not sure what you mean. I request you to please elaborate and provide more information, and also a way to reproduce the problem. |
|
Should I rather place the x button at the bottom right corner of this red message box? |
Add some text to the error message to make it longer, then you will see what is shown in the screenshot above.
No, like in the development warning, the button should be in the top right corner.
|
|
Can you please take a look at the new changes and merge this pr if everything is as expected. |
|
Yes, I first want to set up a way to preview PRs (#46), otherwise I always have to check them out locally. |
|
ok. Let me know if you would want to me to look at anything else in the meantime. |
|
Updated the code to align the two. |
|
fixed |
|
I think you forgot to push, the last commit is 83926e1 which is from 22 hours ago. |
|
Sorry, my bad. pushed it now |
There was a problem hiding this comment.
Not that the vertical space between the button and the top of the box is different to the yellow box.
Please pay attention to these differences.
You could solve this by create an DismissableBox component which has the same styles for positioning text and close buttons and re-use this in both cases.
|
fixed |
use same vertical spacing for both yellow nd red boxes
|
fixed. |
There was a problem hiding this comment.
Ok, it now looks the same. I think this can be further improved in another PR and re-use the Notice component.
| export const Error = ({ type, message }: ErrorInfo) => { | ||
| const [visible, setVisible] = useState(true) | ||
| if (!visible) return null | ||
| else |
There was a problem hiding this comment.
| else |
This is not neccessary. The return in the previous line will end the function execution.
|
Can we merge this one? |
|
Yes, it's approved! |
|
but it has not been merged yet :/ |
|
Can't you do that? |
|
I am not authorized to merge this pull request. |
|
🎉 This PR is included in version 1.21.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.