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

Feat bulk actions UI updates #5461

Merged
merged 3 commits into from
Aug 21, 2019
Merged

Feat bulk actions UI updates #5461

merged 3 commits into from
Aug 21, 2019

Conversation

cshepherd
Copy link
Contributor

Impact: minor
Type: bugfix

Issue

This includes the following UI fixes for Bulk Actions:

  • Navigational dead end when no products match the uploaded CSVs: Instead the UI is reset and a red (error) InlineAlert is shown saying that no products were found.
  • Proper spacing (16px) between filename chips
  • If CSV contains some (but not all) product ids that weren't matched, a red (error) InlineAlert is shown after the green one with the count of products not found.
  • All filtered products are automatically selected

Breaking changes

n/a

Testing

Verify the above are properly implemented and styled

…in an InlineAlert; Handle case where no products found; Chip spacing

Signed-off-by: Christopher Shepherd <[email protected]>
Copy link
Member

@mikemurray mikemurray left a comment

Choose a reason for hiding this comment

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

If I filter by a file with no results sometimes I get this error

Invariant Violation: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
    at invariant (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:199059:15)
    at scheduleWork (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:218920:5)
    at Object.enqueueSetState (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:210173:5)
    at StyledComponent.Component.setState (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:86246:16)
    at StyledComponent.componentWillReceiveProps (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:303309:10)
    at callComponentWillReceiveProps (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:210445:14)
    at updateClassInstance (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:210655:7)
    at updateClassComponent (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:213698:20)
    at beginWork (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:214648:16)
    at performUnitOfWork (http://localhost:3000/packages/modules.js?hash=897806c6e7bfb4e7a703b564805f4bd2ce64eb7a:218316:12)

Other times I get 2 selected when filtering by a file, but there doesn't seem to be any selected products.

@machikoyasuda machikoyasuda self-requested a review August 16, 2019 21:57
…roducts if files removed

Signed-off-by: Christopher Shepherd <[email protected]>
@cshepherd cshepherd requested a review from mikemurray August 20, 2019 02:20
@machikoyasuda machikoyasuda requested a review from willopez August 21, 2019 20:58
@machikoyasuda
Copy link
Contributor

@willopez you can review + merge this! :D

Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

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

Just one minor issue. My prouducts.csv contains three product ids, 1 is an actual product and the other two are just made up. The text in the alert is incorrect, it should say 2 products were not found.

Also,

Seeing this console error:

Warning: Material-UI: the key `leftChip` provided to the classes property is not implemented in ProductGridItems.
You can only override one of the following: isVisible,isHidden.

not_found

Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

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

LGTM

@willopez willopez merged commit fc3ec5f into develop Aug 21, 2019
@kieckhafer kieckhafer mentioned this pull request Aug 28, 2019
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.

4 participants