feature/31 create graphical error notifications #118

itsViggo · 2020-06-30T10:10:44Z

No description provided.

lukefisklennon · 2020-06-30T12:36:03Z

Looks good! There's just a couple of minor things, like do you reckon the contrast with the background is high enough here?

Also I noticed a weird glitch where when you close the notification, the text disappears just before the notification disappears (only for like one frame). 6x slowed down gif of it happening:

These are only very minor things though, we probably don't need to fix them now.

maxwowo

Good attempt, we could improve the error reporting technique though

client/src/api/getCourseInfo.ts

client/src/api/getCoursesList.ts

client/src/api/config.ts

maxwowo · 2020-06-30T14:44:03Z

Looks good! There's just a couple of minor things, like do you reckon the contrast with the background is high enough here?

Also I noticed a weird glitch where when you close the notification, the text disappears just before the notification disappears (only for like one frame). 6x slowed down gif of it happening:

These are only very minor things though, we probably don't need to fix them now.

Sharp eyes! I think the reason the text disappears first before the snackbar goes away is because

There's a single snackbar
The error message that it contains is a state
Whether the snackbar is open or not depends on if the error message it contains is empty or not, empty error message => close snackbar
Closing the snackbar by clicking on the cross calls a handler which sets the error message it contains to be empty, which means the snackbar will close. So the text will always disappear first before the snackbar closes, since it's the precondition for the snackbar to close

I reckon we can fix this issue by using separate variables for snackbar text and error messages

lukefisklennon · 2020-06-30T14:52:57Z

@maxwowo Ahh I see, that makes sense. I'm probably being a bit pedantic, unlikely that anyone will notice. But if it's an easy fix, might as well.

…ears just before the notification dissapears and changed snackbar design for higher contrast with background

maxwowo · 2020-07-07T03:55:16Z

For some reason when I hover over the close button the snackbar would shrink and grow a little bit, any idea what might be causing this?

maxwowo

Looks good

maxwowo · 2020-07-07T05:14:29Z

client/src/App.tsx

      const newSelectedCourses = [...selectedCourses, selectedCourseClasses];
      populateTimetable(selectedCourseClasses); // TODO: temp until auto-timetabling is done
      setSelectedCourses(newSelectedCourses);
+    } catch (e) {
+      console.log('here');


Probably don't need this here

…e/31-create-graphical-error-notifications

maxwowo

Some minor issues here and there :P

maxwowo · 2020-07-14T03:26:43Z

client/src/api/getCourseInfo.ts

+    if (data.status === 400) {
+      throw new NetworkError('Internal server error');
+    }
+    console.log();


Don't need this

client/src/api/getCoursesList.ts

…e/31-create-graphical-error-notifications

maxwowo

I realized that starting with npm start no longer shows the error snackbar but does report the error, does it happen for you too?

created graphical error notifications

9993ddc

itsViggo requested review from maxwowo, lukefisklennon and esyw and removed request for maxwowo June 30, 2020 10:10

maxwowo suggested changes Jun 30, 2020

View reviewed changes

client/src/api/getCourseInfo.ts Outdated Show resolved Hide resolved

client/src/api/getCoursesList.ts Outdated Show resolved Hide resolved

client/src/api/config.ts Outdated Show resolved Hide resolved

changed NetworkError to a custom error, fixed a bug where text disapp…

af67989

…ears just before the notification dissapears and changed snackbar design for higher contrast with background

lukefisklennon approved these changes Jul 2, 2020

View reviewed changes

esyw approved these changes Jul 2, 2020

View reviewed changes

maxwowo suggested changes Jul 7, 2020

View reviewed changes

Merge branch 'dev' of https://github.com/csesoc/notangles into featur…

5f4d28d

…e/31-create-graphical-error-notifications

maxwowo suggested changes Jul 14, 2020

View reviewed changes

itsViggo added 2 commits July 14, 2020 14:21

Fixed a bug where 'Internal server error' never gets called

9d4f80e

Merge branch 'dev' of https://github.com/csesoc/notangles into featur…

339f7c0

…e/31-create-graphical-error-notifications

maxwowo reviewed Jul 18, 2020

View reviewed changes

added TimeoutError

8840cc6

maxwowo approved these changes Jul 21, 2020

View reviewed changes

Merge branch 'dev' into feature/31-create-graphical-error-notifications

e682cbd

itsViggo merged commit 742360a into dev Jul 21, 2020

munjoonteo deleted the feature/31-create-graphical-error-notifications branch February 11, 2022 07:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/31 create graphical error notifications #118

feature/31 create graphical error notifications #118

itsViggo commented Jun 30, 2020

lukefisklennon commented Jun 30, 2020

maxwowo left a comment

maxwowo commented Jun 30, 2020 •

edited

lukefisklennon commented Jun 30, 2020

maxwowo commented Jul 7, 2020 •

edited

maxwowo left a comment

maxwowo Jul 7, 2020

maxwowo left a comment

maxwowo Jul 14, 2020

maxwowo left a comment •

edited

feature/31 create graphical error notifications #118

feature/31 create graphical error notifications #118

Conversation

itsViggo commented Jun 30, 2020

lukefisklennon commented Jun 30, 2020

maxwowo left a comment

Choose a reason for hiding this comment

maxwowo commented Jun 30, 2020 • edited

lukefisklennon commented Jun 30, 2020

maxwowo commented Jul 7, 2020 • edited

maxwowo left a comment

Choose a reason for hiding this comment

maxwowo Jul 7, 2020

Choose a reason for hiding this comment

maxwowo left a comment

Choose a reason for hiding this comment

maxwowo Jul 14, 2020

Choose a reason for hiding this comment

maxwowo left a comment • edited

Choose a reason for hiding this comment

maxwowo commented Jun 30, 2020 •

edited

maxwowo commented Jul 7, 2020 •

edited

maxwowo left a comment •

edited