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

Feature generic table #1786

Closed

Conversation

sagar-joshi
Copy link
Contributor

@sagar-joshi sagar-joshi commented Mar 4, 2021

Fixes #1442

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest develop branch. (If I was asked to make more changes, I have made sure to rebase onto develop then too)
  • is descriptively named and links to an issue number, i.e. Fixes #123

SketchList, CollectionList, AssetList, AddToCollectionList, AddToCollectionSketchList and Collection use the Table component. The Table component has the sorting state, sorting function, searching function. Search state is in redux.

Also I was not able to test the AssetList due to some error (Error: getaddrinfo ENOTFOUND s3) ( at line .get(s3/objects) inside getAssets() ). After this error app used to crash.

hi @catarak please review the changes.

Earlier DashBoardView used to render the Table component for SketchList, CollectionList, AssetsList. Now DashboardView renders  SketchTable, CollectionTable, AssetsTable components and these three render the Table component
Earlier asset rows, sketch rows, collection rows all were rendered in TableRow component, now these rows are rendered in AssetTable, SketchTable, CollectionTable and then passed to Table component
… SketchListRow, move CollectionListRow.jsx to ../
earlier (field and direction) state was used for sorting, now (field, type and direction) state is used
In the current component structure Searchbar is the child of DashboardView and Overlay so if react state is used,the search state will have to be passed through Searchbar -> DashboardView -> ..... -> Table and Searchbar -> Overlay -> ..... -> Table but with redux its more simpler: Searchbar -> redux store, redux store -> Table
…/desc button

use React.Fragment instead of <div> in SketchTable, CollectionTable
…nd 1st data row's background colors were matching in AddToCollectionList and AddToCollectionSketchList due to which it was difficult to differentiate b/w table header and data.
delete reducers/sorting.js, actions/sorting.js had searching actions also so rename sorting.js to search.js and delete sorting actions from it, delete getFilteredCollections and getSortedCollections from selectors/collections.js, delete selectors/projects.js, delete TableRow.jsx
@release-com
Copy link

release-com bot commented Mar 4, 2021

Release Environments

p5.js-web-editor
app-ted687e-p5-js-web-editor.releaseapp.io

@catarak
Copy link
Member

catarak commented May 4, 2021

Thanks for your patience in my getting back to you on this! I've been on a GitHub hiatus and am now reviewing PRs again. I have some suggestions for changes—are you able to work more on this or should I make them?

@sagar-joshi
Copy link
Contributor Author

I will not be able to manage time for this due to other works these days. You can make the changes.

@raclim
Copy link
Collaborator

raclim commented Apr 18, 2023

Thank you so much for taking the time to contribute to this issue—I'm sorry we couldn't get to your work in time! Since some time has passed I'm going to close this for now, but please feel free to reopen this or work on this again, thanks!

@raclim raclim closed this Apr 18, 2023
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.

Create generic Table component
3 participants