Closes #1220: Better error/loading handling when posts can't be loaded #1251

zjjiang2 · 2020-10-29T10:41:48Z

Issue This PR Addresses

From Issue #1220

Type of Change

Bugfix: Change which fixes an issue
New Feature: Change which adds functionality
Documentation Update: Change which improves documentation
UI: Change which improves UI

Description

Added error handling for UX, includes:

Post fails to load
Post is loading
Timeline fails to load

Checklist

Quality: This PR builds and passes our npm test and works locally
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

humphd · 2020-11-04T22:34:48Z

Pinging some reviewers for this. I like where it's headed, but wording might need tweaks.

cindyorangis

This is great :D

PedroFonsecaDEV

Please don't forget to REBASE before and after start working on the requested changes.
Thanks.

PedroFonsecaDEV · 2020-11-08T05:40:21Z

src/frontend/src/components/Post/Post.jsx

@@ -7,6 +7,9 @@ import { Box, Grid, Typography, ListSubheader } from '@material-ui/core';
 import './telescope-post-content.css';
 import AdminButtons from '../AdminButtons';

+import Spinner from '../Spinner/Spinner.jsx';
+import ErrorRoundedIcon from '@material-ui/icons/ErrorRounded';


"@material-ui/icons/ErrorRounded import should occur before import of ../AdminButtonseslintimport/order"

It's a quick fix, linter will handle this for you.

PedroFonsecaDEV · 2020-11-08T05:43:24Z

src/frontend/src/components/Post/Post.jsx

+        </ListSubheader>
+      </Box>
+    );
+  } else if (!post) {


Please use Prettier here.
Step #1 Just a click and you will be good.

You can use the "Quick fix..."

After this, run Prettier to solve the following issue(Quick fix again or Prettier shortcut):

PedroFonsecaDEV · 2020-11-08T05:52:28Z

src/frontend/src/components/Post/Post.jsx

+      </Box>
+    );
+  } else {
+    return (


The same problem of line 114. Follow the same steps and you will be good.

PedroFonsecaDEV · 2020-11-08T05:56:27Z

src/frontend/src/components/Posts/Posts.jsx

@@ -1,16 +1,31 @@
 import React from 'react';
 import { useSWRInfinite } from 'swr';

-import { Container } from '@material-ui/core';
+import { Container, Typography, Grid } from '@material-ui/core';


"'Typography' is declared but its value is never read.ts(6133)
'Typography' is defined but never used.eslintno-unused-vars"

PedroFonsecaDEV · 2020-11-08T05:57:31Z

src/frontend/src/components/Posts/Posts.jsx

 import { makeStyles } from '@material-ui/core/styles';
 import Timeline from './Timeline.jsx';
 import useSiteMetaData from '../../hooks/use-site-metadata';

+import SentimentDissatisfiedRoundedIcon from '@material-ui/icons/SentimentDissatisfiedRounded';


"@material-ui/icons/SentimentDissatisfiedRounded import should occur before import of ./Timeline.jsxeslintimport/order"

Lint will help you with this one too ("Quick fix...")

After fixing the import order, you will notice an error, run prettier again and you will be fine.

PedroFonsecaDEV · 2020-11-08T06:01:58Z

src/frontend/src/components/Posts/Posts.jsx

+      </Container>
+    );
+  } else {
+    return (


"Unnecessary 'else' after 'return'.eslintno-else-return"

Same as before, use "Quick fix..." and then run prettier until solve the indentation problems.

humphd · 2020-11-10T01:03:41Z

@zjjiang2 did you want to finish this soon? You can probably use npm run eslint-fix and npm run prettier to sort out any issues that are left.

…to Issue-1220

PedroFonsecaDEV · 2020-11-10T05:24:58Z

@zjjiang2 We still have some minor fixes about imports order:

1 and 2

Then I think your PR will be good to go.

Thanks.

…to Issue-1220

humphd

Looks good, with one question about theme colours vs. raw colour values.

humphd · 2020-11-10T17:10:29Z

src/frontend/src/components/Posts/Posts.jsx

@@ -11,6 +12,19 @@ const useStyles = makeStyles(() => ({
    padding: 0,
    maxWidth: '785px',
  },
+  error: {
+    position: 'center',
+    color: '#B5B5B5',


@cindyledev, @agarcia-caicedo or someone else who knows our theme code: are there theme values we should be using for these colours instead?

humphd · 2020-11-10T17:10:40Z

src/frontend/src/components/Posts/Posts.jsx

+  },
+  errorIcon: {
+    position: 'center',
+    color: '#B5B5B5',


Same question here re: colours/theme.

PedroFonsecaDEV · 2020-11-11T20:58:43Z

@manekenpix
It's good to go.
Should I click on "Merge pull request" or "Rebase and merge"?

manekenpix · 2020-11-11T21:04:10Z

Rebasing would be ideal so we can keep a linear history, but I don't trust github solving conflicts so I guess we'll have to merge it as it is.

vercel bot deployed to Preview October 29, 2020 10:41 View deployment

manekenpix changed the title ~~Issue 1220~~ Closes #1220: Better error/loading handling when posts can't be loaded Oct 29, 2020

manekenpix assigned zjjiang2 Oct 29, 2020

manekenpix added the area: front-end label Oct 29, 2020

cindyorangis added the hacktoberfest-accepted label Oct 31, 2020

humphd requested review from cindyorangis, agarcia-caicedo and raygervais November 4, 2020 22:34

cindyorangis previously approved these changes Nov 7, 2020

View reviewed changes

PedroFonsecaDEV requested changes Nov 8, 2020

View reviewed changes

zjjiang2 dismissed cindyorangis’s stale review via e53f94f November 10, 2020 04:51

vercel bot deployed to Preview November 10, 2020 04:51 View deployment

Error Handling Display Format Fixes

a131bcd

zjjiang2 force-pushed the Issue-1220 branch from e53f94f to a131bcd Compare November 10, 2020 05:02

vercel bot deployed to Preview November 10, 2020 05:02 View deployment

zjjiang2 added 2 commits November 10, 2020 00:10

Error Handling Display Format Fixes with Pretty

8e86fa4

Merge branch 'Issue-1220' of https://github.com/zjjiang2/telescope in…

f27d6c9

…to Issue-1220

vercel bot deployed to Preview November 10, 2020 05:14 View deployment

zjjiang2 added 3 commits November 10, 2020 02:07

Error Handling Display Format Fixes

591565d

Merge branch 'Issue-1220' of https://github.com/zjjiang2/telescope in…

2601d47

…to Issue-1220

Error Handling Display Format Fixes

16bb9f9

vercel bot deployed to Preview November 10, 2020 07:11 View deployment

humphd approved these changes Nov 10, 2020

View reviewed changes

humphd requested a review from cindyorangis November 10, 2020 23:13

c3ho approved these changes Nov 11, 2020

View reviewed changes

manekenpix requested a review from PedroFonsecaDEV November 11, 2020 17:48

PedroFonsecaDEV approved these changes Nov 11, 2020

View reviewed changes

PedroFonsecaDEV merged commit 7d97026 into Seneca-CDOT:master Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Closes #1220: Better error/loading handling when posts can't be loaded #1251

Closes #1220: Better error/loading handling when posts can't be loaded #1251

zjjiang2 commented Oct 29, 2020 •

edited

Loading

humphd commented Nov 4, 2020

cindyorangis left a comment

PedroFonsecaDEV left a comment

PedroFonsecaDEV Nov 8, 2020

PedroFonsecaDEV Nov 8, 2020

PedroFonsecaDEV Nov 8, 2020 •

edited

Loading

PedroFonsecaDEV Nov 8, 2020

PedroFonsecaDEV Nov 8, 2020

PedroFonsecaDEV Nov 8, 2020

humphd commented Nov 10, 2020

PedroFonsecaDEV commented Nov 10, 2020

humphd left a comment

humphd Nov 10, 2020

humphd Nov 10, 2020

PedroFonsecaDEV commented Nov 11, 2020

manekenpix commented Nov 11, 2020 •

edited

Loading

Closes #1220: Better error/loading handling when posts can't be loaded #1251

Closes #1220: Better error/loading handling when posts can't be loaded #1251

Conversation

zjjiang2 commented Oct 29, 2020 • edited Loading

Issue This PR Addresses

Type of Change

Description

Checklist

humphd commented Nov 4, 2020

cindyorangis left a comment

Choose a reason for hiding this comment

PedroFonsecaDEV left a comment

Choose a reason for hiding this comment

PedroFonsecaDEV Nov 8, 2020

Choose a reason for hiding this comment

PedroFonsecaDEV Nov 8, 2020

Choose a reason for hiding this comment

PedroFonsecaDEV Nov 8, 2020 • edited Loading

Choose a reason for hiding this comment

PedroFonsecaDEV Nov 8, 2020

Choose a reason for hiding this comment

PedroFonsecaDEV Nov 8, 2020

Choose a reason for hiding this comment

PedroFonsecaDEV Nov 8, 2020

Choose a reason for hiding this comment

humphd commented Nov 10, 2020

PedroFonsecaDEV commented Nov 10, 2020

humphd left a comment

Choose a reason for hiding this comment

humphd Nov 10, 2020

Choose a reason for hiding this comment

humphd Nov 10, 2020

Choose a reason for hiding this comment

PedroFonsecaDEV commented Nov 11, 2020

manekenpix commented Nov 11, 2020 • edited Loading

zjjiang2 commented Oct 29, 2020 •

edited

Loading

PedroFonsecaDEV Nov 8, 2020 •

edited

Loading

manekenpix commented Nov 11, 2020 •

edited

Loading