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

#157 Unify errors into a single component #184

Merged
merged 7 commits into from
Mar 4, 2023
Merged

Conversation

kevinzhang03
Copy link
Member

@kevinzhang03 kevinzhang03 commented Mar 4, 2023

Description

#157

Created a new component ErrorGraphic that is responsible for rendering the UI whenever there is an error for GameConsole and FileViewer. It is designed so that it can be used with other components as well as it takes props to display the different parts of the error UI.

Also added a dark highlight colour on whichever file you have open to make it easier to tell which one you're on. Also added proper pluralization when listing how many files are selected/in the clipboard.

image

How Has This Been Tested?

The current errors are rendered by checking for conditions to see if there is an error. I "created" the errors by modifying the conditions so that the component would go to render ErrorGraphic. It looks identical to how it was before, except now that it is done with the ErrorGraphic component instead.

image
image
image

@netlify
Copy link

netlify bot commented Mar 4, 2023

Deploy Preview for lodestone-storybook ready!

Name Link
🔨 Latest commit 4ea29ef
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-storybook/deploys/6402ca6db142e300083353ae
😎 Deploy Preview https://deploy-preview-184--lodestone-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 4, 2023

Deploy Preview for lodestone-dashboard ready!

Name Link
🔨 Latest commit 4ea29ef
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-dashboard/deploys/6402ca6d90f955000861d3ab
😎 Deploy Preview https://deploy-preview-184--lodestone-dashboard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@Ynng Ynng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! just please use a utility to merge the tailwind classnames

src/components/ErrorGraphic.tsx Outdated Show resolved Hide resolved
@kevinzhang03 kevinzhang03 requested a review from Ynng March 4, 2023 02:42
Copy link
Member

@Ynng Ynng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
icons are really small

Copy link
Member

@Ynng Ynng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@kevinzhang03 kevinzhang03 merged commit e04235d into 0.4.3 Mar 4, 2023
@kevinzhang03 kevinzhang03 deleted the 157-unify-error branch March 4, 2023 04:40
@Arcslogger Arcslogger added this to the 0.4.3 milestone Mar 4, 2023
@Arcslogger Arcslogger linked an issue Mar 4, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify error graphics
3 participants