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: provide some state and methods via slot scope #73

Draft
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

mesqueeb
Copy link

@tochoromero this is still a WIP. I have yet to test these changes.

I think it's relatively safe since VTh will always be a direct child to VTable and therefore can be considered tightly coupled. So this is a rare case where it's safe to access parent methods & state imo.

I have added an extra check in the setup to check for the case where VTh is for some reason not a child of VTable.

I will continue to test and update this PR.

@mesqueeb
Copy link
Author

mesqueeb commented Jul 19, 2021

@tochoromero

Check out my branch and run npm run dev.
Then check out the source code for my proof of concept here:
https://github.com/tochoromero/vuejs-smart-table/blob/4c25e76b7662cda692e7c7f07bcb73e9a902c97e/App.vue#L14~L20

See the GIF here:
2021-07-19 20 06 41

Some notes:

  • I added a new prop called canSort for the VTh component, to make it possible to use VTh for other things besides sorting as well. Otherwise it would show a sortable icon by default.
  • I made canSort true by default, in order to have 0 breaking changes.
  • I felt it's cleaner to add a prop called canSort as opposed to not adding any new props but doing something like :sortKey="false" instead. Let me know what you think.

Please tell me if you are OK with this implementation. I will continue the PR appropriately.
If OK to proceed I will...

  • For VTh: create a computed property called allSelected (with get & set syntax) like described in my issue before, where setting this to true will execute selectAll() so it can be linked to a checkbox.
  • For VTr: make similar changes to the VTr component but call it isSelected instead

What do you want me to do with the dev playground file? Shall I delete those changes again before finalising the PR?

@mesqueeb
Copy link
Author

@tochoromero did you see this PR? You've addressed what I tried to achieve with this PR in the latest release, so should I go ahead and close this PR?

PS: a final question for you.
I did research how to best access the Vue parent component instance in Vue 3 and for tightly coupled components came up with this:

    const internalInstance = getCurrentInstance()
     const parentInstance = internalInstance?.parent?.proxy
     if (!parentInstance || parentInstance?.$options.name !== 'VTable') {
       throw new Error('VTh must be used in the header slot of VTable.')
     }

then further down I was passing parts of the parent scope to the slot scope like so:

 children.push(h('span', [slots.default({
           sortOrder: order.value,
           rows: (parentInstance as any).tableState.rows,
           selectedRows: (parentInstance as any).tableState.selectedRows,
           selectAll: () => (parentInstance as any).selectAll(),
           deselectAll: () => (parentInstance as any).deselectAll(),
         })]))

I was hoping to get your thoughts on this, because you used a very different mechanic to achieve the same.

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.

1 participant