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

#1744 Create ReportTable component #1758

Merged
merged 5 commits into from Dec 20, 2019

Conversation

diegoevangelisti
Copy link
Contributor

Fixes #1755

Change summary

Add ReportTable component to use together with ReportRow and ReportCell components for visualising Table reports on the feature/dashboard.

  • It uses React Hooks API.
  • Needed in conjunction with ReportRow and ReportCell components.

Testing

Further testing will be taken once the feature/dashboard is intended to be merged into develop.

Related areas to think about

Codebase taken from past work: branch. Further changes may be taken into consideration.

Copy link
Contributor

@josh-griffin josh-griffin left a comment

Choose a reason for hiding this comment

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

🎉

Nice work :)

Now you can understand the more complicated DataTable component easier! 🎉

export const ReportTable = ({ rows, header }) => {
const renderItem = ({ item, index }) => <ReportRow rowData={item} rowIndex={index} />;

const renderHeader = () => <ReportRow rowData={header} isHeader rowIndex={0} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, but there is also this prop on FlatList just for interest! https://facebook.github.io/react-native/docs/flatlist#listheadercomponent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this change here: 7218d46
Hope it works, otherwise I can come back to previous stage :-) thanks for this!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that should work!

@@ -0,0 +1,37 @@
/* eslint-disable react/prop-types */
Copy link
Contributor

Choose a reason for hiding this comment

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

this is for the renderItem call? Theres some info here about that if you're interested jsx-eslint/eslint-plugin-react#2325

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have a problem with that indeed. If I declared them then es-lint says I'm not using them 😅

@diegoevangelisti diegoevangelisti merged commit 4f7a366 into feature/dashboard Dec 20, 2019
@diegoevangelisti diegoevangelisti deleted the #1744-ReportTable-component branch December 20, 2019 02:10
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.

None yet

2 participants