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: pagination component #356

Merged
merged 14 commits into from
Sep 17, 2022
Merged

feat: pagination component #356

merged 14 commits into from
Sep 17, 2022

Conversation

hackpirodev
Copy link
Collaborator

@hackpirodev hackpirodev commented Sep 10, 2022

related to #325

@codesandbox
Copy link

codesandbox bot commented Sep 10, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@changeset-bot
Copy link

changeset-bot bot commented Sep 10, 2022

⚠️ No Changeset found

Latest commit: e79458a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@hackpirodev hackpirodev temporarily deployed to Preview September 10, 2022 15:51 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Sep 10, 2022

Deploy preview for codeimage ready!

✅ Preview
https://codeimage-93egspjbz-riccardoperra.vercel.app
https://codeimage-pr-356.vercel.app

Built with commit e79458a.
This pull request is being automatically deployed with vercel-action

@riccardoperra riccardoperra temporarily deployed to Preview September 10, 2022 16:17 Inactive
todo: cleanup, style and move as in codeimage ui
@hackpirodev hackpirodev temporarily deployed to Preview September 10, 2022 18:11 Inactive
{page: Accessor<number>; setPage: Setter<number>; pageSize: number},
] => {
const [page, setPage] = createSignal(1),
pageSize = 9;
Copy link
Owner

@riccardoperra riccardoperra Sep 12, 2022

Choose a reason for hiding this comment

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

The page size must be passed as option

export const createPagedData = <T>(
  data: Accessor<T[] | undefined>,
  options: {
    pageSize: number
  }
): [
  Accessor<T[]>,
  {page: Accessor<number>; setPage: Setter<number>; pageSize: number},
]

@riccardoperra riccardoperra changed the title Feat/pagination component feat: pagination component Sep 12, 2022
WIP - clening up
@hackpirodev hackpirodev temporarily deployed to Preview September 12, 2022 21:35 Inactive
@riccardoperra riccardoperra marked this pull request as ready for review September 15, 2022 06:59
trying add inital page to Pagination
@hackpirodev hackpirodev temporarily deployed to Preview September 15, 2022 20:26 Inactive
}),
);
console.log('pageSelected', options.pageSelected);
const [page, setPage] = createSignal(options.pageSelected);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@riccardoperra in the dashboard.state I pass as options.pageSelected = 5. In this console.log I've obtained 5 but the page() of the dashboard it's still 1 do you know why?

clean up and centering the buttons
@hackpirodev hackpirodev temporarily deployed to Preview September 17, 2022 06:42 Inactive
@hackpirodev hackpirodev temporarily deployed to Preview September 17, 2022 12:42 Inactive
Comment on lines 24 to 26
const handleChange = (direction: 'next' | 'prev') => () =>
merged.onChange(prev => (direction === 'next' ? prev + 1 : prev - 1));

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@riccardoperra do you know why in this clousure eslint suggest me to do it in createEffect,? maybe I don't do this?

Copy link
Owner

@riccardoperra riccardoperra Sep 17, 2022

Choose a reason for hiding this comment

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

handle-change is already a callback that does an action, it must not be return another callback. Also, it should be separated into two function that does increment and decrement action instead of one with a string as parameter

move here pagination component

and hook to get paginatedData
@hackpirodev hackpirodev temporarily deployed to Preview September 17, 2022 13:36 Inactive
function makeDashboardState(authState = getAuth0State()) {
const [data, {mutate, refetch}] = createResource(fetchWorkspaceContent, {
initialValue: [],
});

const [search, setSearch] = createSignal('');

const [pageSize] = createSignal(9);
Copy link
Owner

Choose a reason for hiding this comment

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

Non é necessario che sia un signal

@@ -0,0 +1,19 @@
import {Accessor, createSignal, Setter} from 'solid-js';
interface pagedDataOptions {
pageSize: Accessor<number>;
Copy link
Owner

@riccardoperra riccardoperra Sep 17, 2022

Choose a reason for hiding this comment

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

questo dovrebbe essere trattato come number piu che accessor

dall'hook poi per non perdere reattivitá si potrebbe chiamare come se lo fosse

const pageSize = () => options.pageSize

@hackpirodev hackpirodev temporarily deployed to Preview September 17, 2022 14:17 Inactive
Comment on lines +13 to +14
data: Accessor<T[]>,
pageSize: Accessor<number>,
Copy link
Collaborator Author

@hackpirodev hackpirodev Sep 17, 2022

Choose a reason for hiding this comment

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

@riccardoperra qui il pageSize lo vogliamo ancora Accesor no? quindi ora che è un const lo passeremo ()=>pageSize

@hackpirodev hackpirodev temporarily deployed to Preview September 17, 2022 14:26 Inactive
@riccardoperra riccardoperra merged commit 4d2a592 into next Sep 17, 2022
@riccardoperra riccardoperra deleted the feat/pagination-component branch September 17, 2022 14:37
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.

2 participants