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

Improve UI of Error Toasts #11947

Closed
thikhinab opened this issue Aug 23, 2022 · 8 comments · Fixed by #12509
Closed

Improve UI of Error Toasts #11947

thikhinab opened this issue Aug 23, 2022 · 8 comments · Fixed by #12509

Comments

@thikhinab
Copy link
Contributor

  • Environment: master branch at commit 39192f4

Description of feature/enhancement
Propose to update the front-end to facilitate error messages to be represented as multiple toasts rather than a single one.

Justification
Currently if there are multiple error messages, it appears in a single toast in certain cases (e.g. where the back-end returns an array of error messages). Since the messages are displayed as an array it reduces the legibility/understanding of errors. Furthermore, the users may also view it as a bug since it seems like the array of messages is returned without being processed.

Image below shows an instance where there are three error messages:
image

@damithc
Copy link
Contributor

damithc commented Aug 23, 2022

Yes, it might be better to show as three separate lines (like a bullet list). I'm not sure how much formatting is possible inside toasts though.

@thikhinab
Copy link
Contributor Author

@damithc Okay prof, will start working on this.

@domoberzin
Copy link
Contributor

Hi, is this issue still active? If so, may I work on it?

@weiquu
Copy link
Contributor

weiquu commented Jul 2, 2023

Hi @domoberzin, sure please feel free to submit a PR for this

@domoberzin
Copy link
Contributor

Was attempting to fix this, following the conversation in this PR, but realised that by changing the whitespace property to pre-wrap or pre-line:

  1. Some E2E tests fail due to new \n characters in the actual status message, compared to the expected
  2. Visual appearance of longer messages in the toast are also formatted weirdly as seen in the image below

image

With all this in mind, I want to ask if there is perhaps a different way to display the error messages? One way I could think of would be to display the errors with different separators, for e.g.
Simply displaying it as a comma separated list without the array brackets or Error 1 || Error 2 || Error 3

@weiquu
Copy link
Contributor

weiquu commented Jul 12, 2023

Hi @domoberzin, the issue with the toast message above seems to be caused by lines 804-805 in instructor-course-edit-page.component.ts. I combined them into a single line and the toast message looked fine for me.

Could you also try setting the whitespace property to break-spaces and see if the E2E tests still fail? If so, then I think we can just go with your suggestion.

@domoberzin
Copy link
Contributor

Hello @weiquu, I've worked on it and seem to have found a way to fix the E2E errors by closing the toast after the message within it has been verified, and also by editing the messages to be formatted correctly. They can be found in this PR #12509.

Also, I want to mention that while working on this issue, I found that some E2E test errors, particularly those relating to toast, for example in this test run, were not very specific, since it says that the toast component cannot be found, but the actual error was that the status message did not match with the expected message. Should I open an issue for this?

@weiquu
Copy link
Contributor

weiquu commented Jul 13, 2023

Thanks @domoberzin! Sure, feel free to open an issue for it

weiquu pushed a commit that referenced this issue Jul 13, 2023
* fix: format error toasts

* fix: css for line breaks

* fix: change formatting for certain toast messages

* fix: lint errors

* fix: add wait time

* fix: close toast after verification

* fix: lint errors

* fix: formatted status message

* fix: add whitespace to status message

* fix: remove extra inverted commas

---------

Co-authored-by: Jason Qiu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants