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: Can't toggle off from the ad-hoc tools menu if you've toggled on in DetailsView #7431

Merged
merged 6 commits into from
Aug 29, 2024

Conversation

madalynrose
Copy link
Contributor

Details

Two ad-hoc tools (Automated Checks and Needs Review) have the ability to view issues in DetailsView. Unfortunately, if you try to toggle either of them off in ad-hoc tools while their corresponding DetailsView list of issues is open, the DetailsView toggle will overwrite the "off" state and force it to be "on". Additionally, toggling off in DetailsView does not turn the toggle off in ad-hoc tools.

This PR keeps these toggles in sync by:

  • moving the code that determines if a scan needs to be run based on state to componentDidMount instead of on render.
  • triggering VisualizationActions.enableVisualization/VisualizationActions.disabledVisualization when CardSelectionActions.toggleVisualHelper is triggered, which involves:
    • making sure VisualizationActions are accessible from CardSelectionActionCreators
    • sending a VisualizationTogglePayload instead of BasePayload with the enabled state (passed in from the VisualHelperToggle's onClick)
  • triggering CardSelectionActions.toggleVisualHelper when VisualizationActions.enableVisualization/VisualizationActions.disabledVisualization are triggered, which involves:
    • checking the value of payload.test and triggering needsReviewSelectionActions.toggleVisualHelper or cardSelectionActions.toggleVisualHelper if the VisualizationType matches.
Motivation

Addresses issue #6253

Context

Pull request checklist

  • Addresses an existing issue: Can't toggle off from the ad-hoc panel if you've toggled on in DetailsView #6253
  • Added/updated relevant unit test(s) (and ran yarn test)
  • Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • PR title AND final merge commit title both start with a semantic tag (fix:, chore:, feat(feature-name):, refactor:). See CONTRIBUTING.md.

@madalynrose madalynrose requested a review from a team as a code owner August 27, 2024 15:04
@madalynrose madalynrose merged commit 9c0dafa into microsoft:main Aug 29, 2024
12 checks passed
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