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

Add a way to limit the ParallelDOM API to just accessibleName and helpText #1666

Closed
jessegreenberg opened this issue Oct 28, 2024 · 4 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

Many components will should only allow setting accessibleName and helpText of ParallelDOM options. Lets use TypeScript to make this clear. I am imagining a type that will basically omit all ParallelDOMOptions except for these two.

@jessegreenberg
Copy link
Contributor Author

I added a parametric type called TrimmedParallelDOMOptions. It omits all but accessibleName and helpText. It can be used like this:

export type ABSwitchOptions = TrimmedParallelDOMOptions<SelfOptions & HBoxOptions>;

@pixelzoom can you please review this, what are your thoughts about using this in common code to constrain the API?

@pixelzoom
Copy link
Contributor

pixelzoom commented Oct 29, 2024

Re TrimmedParallelDOMOptions... @jessegreenberg and I discussed.

I pointed out that Node handles this a bit differently. It exports subsets of options, like NodeTranslationOptions and NodeTransformOptions, and composes those to form NodeOptions. Clients can then use those subsets. But I don't know if that's a viable approach for things that inherit from ParallelDOM.

I'm unsure whether 1 subset of high-level options (currently 'accessibleName' | 'helpText' | 'focusable' | 'pdomVisible') will be appropriate in all cases, or whether (more likely) it will vary by UI component.

I'm unsure if TrimmedParallelDOMOptions is the best name, but I can't think of anything better. I did mention that other utility types tend to be named with verbs (Pick, Omit,...) so TrimParallelDOMOptions might be better.

@jessegreenberg
Copy link
Contributor Author

Thanks for the thoughts here @pixelzoom, I considered these a bit more.

I pointed out that Node handles this a bit differently

Doing it this way will be difficult because NodeOptions extends ParallelDOMOptions. I still want to be able to use all ParallelDOMOptions with Node. And so I can't think of a pattern that will support that without omit.

I'm unsure whether 1 subset of high-level options

Yes, we will probably need others, but I think this is the right starting set for UI components as we scale out accessible names and help text. Variations can include or omit things as they need and if a pattern forms we can create a new subset.

m unsure if TrimmedParallelDOMOptions is the best name,

Good point, Ill rename it to TrimParallelDOMOptions.

@jessegreenberg
Copy link
Contributor Author

The rename is done. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants