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

Replace automatic option enable/disable with a list of requirements #101

Merged
merged 4 commits into from
Feb 10, 2025

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Feb 10, 2025

This may be somewhat of a UX regression, as we no longer enable/disable options automatically, but instead dynamically change whether an item is active or not. We could still reintroduce the automatic requirement selection (which I think may be slightly confusing), but that is a tricky problem to solve now, especially if options get into a requires-requires not cycle.

I'm slightly worried about the regression, because now if we have exclusive options, those don't deselect each other, but the user has to manually toggle them. This also means that the "neither one selected" must be a valid option, although even the previous method didn't handle "option A enables option B enables option C" recursive cases so maybe this is just trading limitations.

The goal of this PR is to enable "Allow unstable HAL features" and "Allow unstable dependencies" options.

After this, we could probably perma-hide features that are not available on the current chip. Having an option suggests it can be enabled somehow.

@bugadani bugadani marked this pull request as ready for review February 10, 2025 14:25
Copy link
Collaborator

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

With the hint showing why a disabled option is disabled I think it's not too bad UX-wise

The other way around (e.g. trying to disable alloc while wifi is enabled) is a bit less ergonomic - but not a big deal with the amount of options we currently have

@bjoernQ bjoernQ added this pull request to the merge queue Feb 10, 2025
@bugadani
Copy link
Contributor Author

I'm hoping we can auto-generate the requires/required by helper texts, so that should improve the situation a bit.

Merged via the queue into main with commit f7d1559 Feb 10, 2025
19 checks passed
@bugadani bugadani deleted the requires branch February 10, 2025 16:07
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