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 request] Expose visible computed property from useKBarState #3

Open
MCYouks opened this issue Jan 20, 2022 · 2 comments
Open
Labels
enhancement New feature or request

Comments

@MCYouks
Copy link

MCYouks commented Jan 20, 2022

Hi there,

Thank you very much for your amazing work. I have a request to simplify the usage of useKBarState.

If we want to open the KBar manually, we have to create computed and methods.

It would be great to be provided with a set of computed and methods out of the box.

Current situation

const state = useKBarState();
const visible = computed(() => state.value.visibility !== "hidden");
const open = () => { state.value.visibility = "visible" }
const close = () => { state.value.visibility = "hidden" }

return { visible, open, close }

Ideally

const { visible, open, close } = useKBarState();

return { visible, open, close }

What do you think 🤔

@LiuJi-Jim LiuJi-Jim added the enhancement New feature or request label Jan 26, 2022
@LiuJi-Jim
Copy link
Collaborator

LiuJi-Jim commented Jan 26, 2022

Now we have provided

const handler = useKBarHandler()
// handler.value.show()
// handler.value.hide()
// handler.value.toggle()

so you don't not need to implement open/close shortcut functions by mutating state.value.visibility directly.

The visibility is defined to be type VisualState = "entering" | "visible" | "leaving" | "hidden" preparing for animation handling in the future.
Considering different definition of "visible" (e.g. maybe visibility !== "hidden", or maybe visibility === "visible") in different applications and scenes, I'm hesitating to provide a real "visible" property.
But I do know that might be really convenient. So plz let me take more time thinking about it 😂.

Thanks for advising!

@MCYouks
Copy link
Author

MCYouks commented Jan 26, 2022

Thanks for your reply 🙏

Great enhancement 👌

Now we have provided

const handler = useKBarHandler()
// handler.value.show()
// handler.value.hide()
// handler.value.toggle()

However, if I may suggest, I believe it would be easier to expose the functions directly. For example:

const { show, hide, toggle } = useKBarHandler()

The concept of destructuring objects provides many great benefits, such as more explicit and intentional code.

———

Regarding the visibility state:

The visibility is defined to be type VisualState = "entering" | "visible" | "leaving" | "hidden" preparing for animation handling in the future. Considering different definition of "visible" (e.g. maybe visibility !== "hidden", or maybe visibility === "visible") in different applications and scenes, I'm hesitating to provide a real "visible" property. But I do know that might be really convenient. So plz let me take more time thinking about it 😂.

What about providing a way to customize the <Transition /> state? 🤔

We could provide users with a binary value for the visible state (e.g. true or false), and let them customize enter and leaving transitions with CSS using the transition classes.

What do you think? 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants