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 channel flag to de-select by default on versions page #1255

Merged
merged 2 commits into from
May 4, 2024

Conversation

Machine-Maker
Copy link
Member

Unsure about the name, or maybe it should be defaulted to on and flipped?

Comment on lines 27 to 18
const frozen = props.channel && props.channel.flags.includes(ChannelFlag.FROZEN);
const possibleFlags = frozen ? [ChannelFlag.PINNED] : [ChannelFlag.UNSTABLE, ChannelFlag.PINNED];
const possibleFlags = frozen ? [ChannelFlag.PINNED] : [ChannelFlag.UNSTABLE, ChannelFlag.PINNED, ChannelFlag.HIDE_BY_DEFAULT];

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really a fan of this hardcoded flags based on frozen. Probably should be part of the backend data, what channel flags are valid when frozen or not, but that's a separate PR.

Copy link
Member

@kennytv kennytv left a comment

Choose a reason for hiding this comment

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

Should be fine without being flipped, so that all options are unticked by default. Can you rebase and add an icon with the master changes? something like a crossed out eye

@MiniDigger MiniDigger force-pushed the feature/channel-deselect-flag branch from b02e63d to 5356b81 Compare May 4, 2024 10:12
@MiniDigger MiniDigger force-pushed the feature/channel-deselect-flag branch from 5356b81 to 5c07353 Compare May 4, 2024 10:18
@MiniDigger MiniDigger merged commit 55c19ae into staging May 4, 2024
1 check passed
@MiniDigger MiniDigger deleted the feature/channel-deselect-flag branch May 4, 2024 10:20
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.

3 participants