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

fix: LaunchDevly: Cleanup code and remove only show overrides when there are no overrides #427

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

cdelst
Copy link
Contributor

@cdelst cdelst commented Sep 12, 2024

Previously you could get stuck in an error state where the "only show overrides" checkbox would get locked to on, and then disabled so that you couldn't edit it again due to having no overrides.

I solved this by adding a cheap useEffect hook that checks for this condition and directs us into a valid state.

I thought about hooking this in in various places to avoid the useEffect usage, but a global watcher for this component made the most sense to me.

@cdelst cdelst changed the title fix: Cleanup code and remove only show overrides when there are no overrides fix: LaunchDevly: Cleanup code and remove only show overrides when there are no overrides Sep 12, 2024
}),
);

// Winnow out removed overrides and update local state in a
// single pass
const updatedOverrides = overrideKeys.reduce(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out this was entirely unnecessary

const flagsPerPage = 20;

const overridesPresent = useMemo(
() => overrides && Object.keys(overrides).length > 0,
[overrides],
);

useEffect(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Main bug fix here.

Base automatically changed from cdelst/sc-255978/pagination-functionality-bug to main September 13, 2024 22:57
@cdelst cdelst force-pushed the cdelst/sc-255977/remove-override-bug branch from c99294f to 0ec83ca Compare September 13, 2024 22:57
@cdelst cdelst merged commit 860b657 into main Sep 13, 2024
7 checks passed
@cdelst cdelst deleted the cdelst/sc-255977/remove-override-bug branch September 13, 2024 22:58
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