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: add MenuSeparator component #115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alvaro-cuesta
Copy link

Extracted from this export:

function x(e) {
    const t = o.useContext(k).styles ?? v();
    return o.createElement("div", {
        className: t.ContextMenuSeparator
    })
}

It can be used as a child of Menu, i.e. sibling to MenuItem (see arrows in screenshot).

Screenshot

@PartyWumpus
Copy link
Member

PartyWumpus commented Dec 28, 2024

This is great, although I think we generally try and avoid matching on class names because valve is fickle and changes then often. Although looking at it I suppose you don't really have any other properties to match...
Maybe use findClass, should be slightly more stable?

@alvaro-cuesta
Copy link
Author

alvaro-cuesta commented Dec 28, 2024

@PartyWumpus thanks for looking into this!

Yeah, the component is pretty barebones and there's not a lot to match on 🫤 but I figured it was better to still extract the component rather than using the class manually in a div (in case Valve eventually changes the internal implementation, or adds some props to the component or whatever...)

Maybe use findClass, should be slightly more stable?

Can you elaborate further?

If I understand that correctly findClass gets a classnames module id and a classname on that module? The classnames module is exposed in gamepadContextMenuClasses but I'd still have to specify the classname literal, right? Isn't that the unstable part anyways?

Also, doesn't findClass return the post-bundled hashed classname? The component uses the pre-bundle classname anyways.

@PartyWumpus
Copy link
Member

Also, doesn't findClass return the post-bundled hashed classname? The component uses the pre-bundle classname anyways.

Good point... Yeah this seems like the most reasonable way to match it then.

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