Skip to content

feat: Create an ErrorView that can be used to display errors#1965

Merged
mofojed merged 14 commits intodeephaven:mainfrom
mofojed:error-view
Apr 30, 2024
Merged

feat: Create an ErrorView that can be used to display errors#1965
mofojed merged 14 commits intodeephaven:mainfrom
mofojed:error-view

Conversation

@mofojed
Copy link
Copy Markdown
Member

@mofojed mofojed commented Apr 25, 2024

  • Added to the StyleGuide as well
  • Displays errors in a textarea, with a copy button and an expand button

mofojed added 2 commits April 25, 2024 17:01
- Added to the StyleGuide as well
- Displays errors in a textarea, with a copy button and an expand button
@mofojed mofojed requested a review from dsmmcken April 25, 2024 21:15
@mofojed mofojed self-assigned this Apr 25, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2024

Codecov Report

Attention: Patch coverage is 15.78947% with 16 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (main@c6bcc15). Click here to learn what that means.

❗ Current head f07cdb8 differs from pull request most recent head a050800. Consider uploading reports for the commit a050800 to get more accurate results

Files Patch % Lines
packages/components/src/ErrorView.tsx 0.00% 16 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1965   +/-   ##
=======================================
  Coverage        ?   46.28%           
=======================================
  Files           ?      663           
  Lines           ?    38612           
  Branches        ?     9780           
=======================================
  Hits            ?    17871           
  Misses          ?    20688           
  Partials        ?       53           
Flag Coverage Δ
unit 46.28% <15.78%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mofojed added 7 commits April 26, 2024 09:40
- Make it so the title of the error will truncate if necessary
- Expand button only shows if the error doesn't fit in the small view
- Makes it grow more than necessary when expanded
Copy link
Copy Markdown
Contributor

@dsmmcken dsmmcken left a comment

Choose a reason for hiding this comment

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

webkit styleguide snapshot looks wrong

Comment thread packages/components/src/ErrorView.scss Outdated
Comment thread packages/components/src/ErrorView.scss Outdated
Comment thread packages/components/src/ErrorView.tsx Outdated
Comment thread packages/components/src/ErrorView.tsx Outdated
@mofojed mofojed requested a review from dsmmcken April 29, 2024 18:05
- Needed to update snapshots after merging the latest
- Fonts were a little bit different
Comment thread packages/components/src/ErrorView.scss Outdated
Comment on lines +56 to +60
color: $black;
opacity: 0.8;
padding: $spacer-1;
&:active {
color: $black;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
color: $black;
opacity: 0.8;
padding: $spacer-1;
&:active {
color: $black;
color: var(--dh-color-contrast-dark);
opacity: 0.8;
padding: $spacer-1;
&:active {
color: var(--dh-color-contrast-dark);

Got to watch those color variables for light theme.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We could try adding a stylelint rule to ban sass variables basically. I think we could use this rule and set the regex to something like /iWillNotUseSassVariables/ which would effectively ban sass variables I think

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Would need rules to allow for $spacer as well.

@mofojed mofojed merged commit 65ef1a7 into deephaven:main Apr 30, 2024
@mofojed mofojed deleted the error-view branch April 30, 2024 18:00
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants