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

Pagination, Eslint & Context refactor #38

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Pagination, Eslint & Context refactor #38

wants to merge 30 commits into from

Conversation

RyRy79261
Copy link
Collaborator

closes #7

This PR additionally refactors business logic into two context hooks, one pertaining to Bundles & Tokens

The idea is that since bundles are clusters of blocks, token flow data & any nested block related data, that it would be managed there.

Consuming or interpreting transactions, tokens, balances or balance related events would be managed in the tokens context

Additionally components have been broken out into smaller .tsx components.To make better use of the context hook data being accessable throughout the app, a move towards TS interfaces for data has been made.

Functionally nothing has changed other than some logic for sniffing out if there are more bundles to paginate through

Improvement notes

A simple, can next page, approach has been taken but loading multiple pages and reducing calls can be improved from here

@RyRy79261 RyRy79261 changed the title Pagination Pagination & Context refactor Jun 26, 2021
@RyRy79261 RyRy79261 self-assigned this Jun 26, 2021
@RyRy79261 RyRy79261 changed the title Pagination & Context refactor Pagination, Eslint implementation & Context refactor Jun 26, 2021
@RyRy79261 RyRy79261 changed the title Pagination, Eslint implementation & Context refactor Pagination, Eslint & Context refactor Jun 26, 2021
@RyRy79261 RyRy79261 marked this pull request as ready for review June 26, 2021 12:27
@RyRy79261 RyRy79261 requested a review from martriay June 26, 2021 12:32
Copy link
Owner

@martriay martriay left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR! We're in good track. I left several comments.

  • This looks more like 3 PRs (Pagination, Eslit, Context refactor), please send three separate ones next time
  • The build is not passing:
9:33:10 AM: Error occurred prerendering page "/Home". Read more: https://nextjs.org/docs/messages/prerender-error
9:33:10 AM: Error: useBundleData must be used within a BundleDataProvider
9:33:10 AM:     at useBundleData (/opt/build/repo/.next/server/chunks/952.js:202:11)

.eslintrc.json Outdated Show resolved Hide resolved
.env.template Outdated Show resolved Hide resolved
components/Address.tsx Outdated Show resolved Hide resolved
components/Address.tsx Outdated Show resolved Hide resolved
components/bundles/Subbundle.tsx Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
pages/Home.tsx Outdated Show resolved Hide resolved
pages/index.tsx Outdated Show resolved Hide resolved
pages/Home.tsx Outdated Show resolved Hide resolved
pages/index.tsx Outdated Show resolved Hide resolved
@RyRy79261 RyRy79261 closed this Jun 28, 2021
@RyRy79261 RyRy79261 reopened this Jun 29, 2021
@RyRy79261 RyRy79261 requested a review from martriay June 29, 2021 10:41
@RyRy79261 RyRy79261 added the Status: Do Not Merge 🛑 Used for when PR is valid but deployment needs adjustment label Jun 30, 2021
@RyRy79261
Copy link
Collaborator Author

There seems to be an issue between the CSS being served when running yarn dev & when building the project. I suspect that the bundle module css file is being skipped when the build is being run though unable to verify.

There are tailwind classes that aren't being included in the build css despite being used in the markup, I suspect that tailwind might have a treeshaking build step that might be missing the additional modules though still need to validate that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Do Not Merge 🛑 Used for when PR is valid but deployment needs adjustment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pagination
2 participants