Skip to content

feat(table): implement loading state - #2580

Merged
gergelyke merged 24 commits into
uber:masterfrom
afzalsayed96:infinite_loader
Jan 24, 2020
Merged

feat(table): implement loading state#2580
gergelyke merged 24 commits into
uber:masterfrom
afzalsayed96:infinite_loader

Conversation

@afzalsayed96

@afzalsayed96 afzalsayed96 commented Jan 7, 2020

Copy link
Copy Markdown
Contributor

Related to #2402

Description

Change list
  1. Adds an infinite loading state for progress-bar inspired by material-ui's indeterminate LinearProgress component
  2. Display infinite progress-bar in table controlled by isLoading prop

Scope

  • Patch: Bug Fix
  • Minor: New Feature
  • Major: Breaking Change

@vercel

vercel Bot commented Jan 7, 2020

Copy link
Copy Markdown

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/uber-ui-platform/baseweb/jy2vhxp63
✅ Preview: https://baseweb-git-fork-afzalsayed96-infiniteloader.uber-ui-platform.now.sh

@claassistantio

claassistantio commented Jan 7, 2020

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@chasestarr

Copy link
Copy Markdown
Collaborator

Thanks @afzalsayed96 I'm going to ask our designers to review the loading state. They may ask for some edits to the UX. Issue #2402 is for the data-table component that I plan on taking a slightly different direction on.

@afzalsayed96

Copy link
Copy Markdown
Contributor Author

Thanks for the update @chasestarr . It completely makes to have the design reviewed.

@chasestarr chasestarr left a comment

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.

Design likes the changes. Thank you. A couple of requests, but once addressed this will be ready to merge

Comment thread src/progress-bar/styled-components.js Outdated
Comment thread src/progress-bar/types.js Outdated
Comment thread src/table/table.js Outdated
Comment thread src/table/table.js Outdated
@afzalsayed96

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback. Much cleaner now 👍

@nadiia nadiia left a comment

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.

Looks good other than a comment! Thanks for contributing!

Comment thread src/progress-bar/progressbar.js Outdated
@afzalsayed96

Copy link
Copy Markdown
Contributor Author

Here we go

@chasestarr

Copy link
Copy Markdown
Collaborator

jest fails due to a snapshots being out of sync. Can you please run yarn test -u and commit the result?

@afzalsayed96

Copy link
Copy Markdown
Contributor Author

I was facing some issues running yarn test -u on windows. Specifically when it calls yarn validate:examples command. Running node ./scripts/validate-examples.js manually worked. (Use cross-spawn?)

Also I updated scripts/validate-examples.js to be platform independent when splitting by new lines separator.

@gergelyke gergelyke added the ci label Jan 21, 2020
@gergelyke

Copy link
Copy Markdown
Contributor

@afzalsayed96 can you contribute your changes too, maybe in a new PR that would solve the issues you faced on windows?

@chasestarr chasestarr added ci and removed ci labels Jan 21, 2020
@gergelyke gergelyke added ci and removed ci labels Jan 21, 2020
@gergelyke
gergelyke merged commit 097529c into uber:master Jan 24, 2020
VladimirMilenko pushed a commit to VladimirMilenko/baseui that referenced this pull request Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants