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 state surfacing support #25

Merged
merged 9 commits into from
Jan 6, 2025

Conversation

arkadiuszbachorski
Copy link
Collaborator

@arkadiuszbachorski arkadiuszbachorski commented Dec 3, 2024

Improve state surfacing support

♻️ Current situation & Problem

Design system needs to establish a consistent and easy way to support operations states: loading, error, empty, success.

⚙️ Release Notes

  • Add Badge component
  • Add ErrorState component
  • Add Spinner component
  • Add StateContainer component
  • Add FormError component
  • Add Async component
  • Surface states on SignInForm
  • Add dev utils
  • Add query utils
  • Migrate DataTable to use Async component and simplify it's implementation

StanfordBDHG/ENGAGE-HF-Web-Frontend#95 uses all the introduced elements.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@arkadiuszbachorski arkadiuszbachorski force-pushed the arek/add-error-state-component branch from 65422b9 to 814840d Compare December 3, 2024 11:42
@arkadiuszbachorski arkadiuszbachorski added the enhancement New feature or request label Dec 3, 2024
Copy link
Collaborator

@NikolaiMadlener NikolaiMadlener left a comment

Choose a reason for hiding this comment

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

Thanks for all the additions @arkadiuszbachorski, looks great!
Left some smaller comments.

src/components/Async/Async.tsx Show resolved Hide resolved
src/components/Async/Async.tsx Show resolved Hide resolved
src/components/DataTable/DataTable.test.tsx Outdated Show resolved Hide resolved
src/components/ErrorState/ErrorState.tsx Show resolved Hide resolved
<div
className={cn(
'flex-center',
padding && 'py-8',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really want to hardcode this to 8?

Copy link
Collaborator Author

@arkadiuszbachorski arkadiuszbachorski Jan 2, 2025

Choose a reason for hiding this comment

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

Most definitely! StateContainer aims to standarize spacing between elements for any state.

There are two most common way to use it:

  1. Without grow - in that scenario, it just relies on the padding
  2. With grow - it expands to fulfill the container, but still has the padding.

If custom padding is necessary, they can provide custom container or remove it with padding={false} prop. There is even already case for that in the design system - FormError component. Nevertheless, by providing defaults we keep it consistent for most usages.

src/forms/FormError/FormError.test.tsx Outdated Show resolved Hide resolved
src/forms/FormError/FormError.test.tsx Outdated Show resolved Hide resolved
src/forms/FormError/FormError.tsx Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
Copy link

codecov bot commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 85.40373% with 47 lines in your changes missing coverage. Please review.

Project coverage is 81.13%. Comparing base (4282aee) to head (08064c6).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/utils/dev/dev.ts 6.25% 15 Missing ⚠️
src/modules/auth/SignInForm/SignInForm.tsx 47.62% 11 Missing ⚠️
src/index.ts 0.00% 8 Missing ⚠️
src/components/DataTable/DataTableTableView.tsx 88.38% 5 Missing ⚠️
src/components/Async/Async.tsx 95.00% 2 Missing ⚠️
src/forms/useForm/useForm.ts 83.34% 2 Missing ⚠️
src/utils/dev/index.ts 0.00% 1 Missing and 1 partial ⚠️
...SignInForm/EmailPasswordForm/EmailPasswordForm.tsx 88.89% 1 Missing ⚠️
src/molecules/DashboardLayout/DashboardLayout.tsx 95.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
- Coverage   81.21%   81.13%   -0.08%     
==========================================
  Files         125      139      +14     
  Lines        2437     2638     +201     
  Branches      278      316      +38     
==========================================
+ Hits         1979     2140     +161     
- Misses        437      477      +40     
  Partials       21       21              
Files with missing lines Coverage Δ
src/components/Async/Async.utils.ts 100.00% <100.00%> (ø)
src/components/Async/index.tsx 100.00% <100.00%> (ø)
src/components/Badge/Badge.tsx 100.00% <100.00%> (ø)
src/components/Badge/index.tsx 100.00% <100.00%> (ø)
src/components/DataTable/DataTable.mocks.ts 100.00% <100.00%> (ø)
src/components/DataTable/DataTable.tsx 100.00% <100.00%> (ø)
src/components/DataTable/index.tsx 100.00% <ø> (ø)
src/components/EmptyState/EmptyState.tsx 100.00% <100.00%> (ø)
src/components/ErrorState/ErrorState.tsx 100.00% <100.00%> (ø)
src/components/ErrorState/index.tsx 100.00% <100.00%> (ø)
... and 19 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4282aee...08064c6. Read the comment docs.

@arkadiuszbachorski arkadiuszbachorski merged commit bf01cf7 into main Jan 6, 2025
9 checks passed
@arkadiuszbachorski arkadiuszbachorski deleted the arek/add-error-state-component branch January 6, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants