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.

Sign up for GitHub

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

Jump to bottom

Can't toggle off from the ad-hoc panel if you've toggled on in DetailsView #6253

Open
JGibson2019 opened this issue Dec 7, 2022 · 4 comments
Open

Can't toggle off from the ad-hoc panel if you've toggled on in DetailsView #6253

JGibson2019 opened this issue Dec 7, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@JGibson2019
Copy link
Contributor

Describe the bug

When visualizations are still considered "on" in the DetailsView panel, you cannot toggle them off from the Ad-hoc panel.

To Reproduce
Steps to reproduce the behavior:

  1. Go to any site (used a local file URL for this one)
  2. Click on the Accessibility Insights extension and select "Ad hoc tools" from the pop up
  3. Toggle on "Needs review"
  4. Click "List view and filtering"
  5. Try to Toggle off "Needs review". See it is stuck in the On state

CodePen repro example

Expected behavior

Clicking the toggle should turn the visualization off and update the DetailsView to show the visual helper is off.

Screenshots

Context (please complete the following information)

  • OS Name & Version: Windows 11 Enterprise Version 10.0.22621 Build 22621
  • AI-Web Version & Environment: Production 2.35.0
  • Browser Version: Google Chrome Version 108.0.5359.95
  • Target Page: /accessibility-insights-web/src/tests/end-to-end/test-resources/all.html (from the Web repo)

Are you willing to submit a PR?

Did you search for similar existing issues?

Yes

Additional context
@JGibson2019 JGibson2019 added the bug Something isn't working label Dec 7, 2022
@ghost ghost added the status: new This issue is new and requires triage by DRI. label Dec 7, 2022
@ghost ghost assigned codeofdusk Dec 7, 2022
@codeofdusk codeofdusk added status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. and removed status: new This issue is new and requires triage by DRI. labels Dec 7, 2022
@ghost ghost assigned ferBonnin Dec 7, 2022
@ghost
Copy link

ghost commented Dec 7, 2022

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

@ferBonnin
Copy link
Contributor

this is an edge case. Let's timebox to 4 hours and close otherwise

@ferBonnin ferBonnin added the status: ready for work This issue is ready to be worked on. label Dec 13, 2022
@ghost ghost removed the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label Dec 13, 2022
@sfoslund
Copy link
Member

sfoslund commented Jan 4, 2023

I did some preliminary investigation on this and it looks like the root cause is:

  • When the user disables the toggle in ad hoc tests, the test gets disabled as expected
  • When the details view re-renders with these changes, it triggers this check: https://github.com/microsoft/accessibility-insights-web/blob/main/src/DetailsView/components/issues-table.tsx#L108, which causes us to send a enableFastPassVisualHelperWithoutScan message, which results in us re-enabling the test
  • Since the test has been reenabled, the ad hoc view toggle re-toggles to reflect this change.
  • In some cases this can happen so quickly that it appears that the toggle never turned off at all.

There seem to be a few possible fixes, I'm interested in other people's thoughts:

  • In order to get the behavior described in the original issue (Clicking the toggle should turn the visualization off and update the DetailsView to show the visual helper is off.), I think we would have to change how we store ad hoc test data significantly, which seems like more trouble than it's really worth.
  • When we disable the test in ad hoc panel, also show the "no content available" page in any associated details views
  • When we disable the test in ad hoc panel, close any associated details views

@lisli1
Copy link
Contributor

lisli1 commented Feb 3, 2023

Seeing something similar with the following repro steps:

  1. Turn on Automated checks from Ad hoc tools
  2. Go to Settings and toggle "Enable high contrast" (either off or on)
  3. Try to turn off Automated checks from Ad hoc tools
  4. The Automated checks toggle does toggle off but it immediately toggles back on

@DaveTryon DaveTryon moved this from Needs Triage to Old backlog in Accessibility Insights Jun 28, 2023
@DaveTryon DaveTryon moved this from Old backlog to Accepted in Accessibility Insights Jun 28, 2023
@DaveTryon DaveTryon added active (Dev) The assigned dev is working to perform the work. and removed status: ready for work This issue is ready to be worked on. labels Jun 30, 2023
@DaveTryon DaveTryon removed the active (Dev) The assigned dev is working to perform the work. label Nov 20, 2023
@madalynrose madalynrose mentioned this issue Aug 27, 2024
Merged
4 tasks
madalynrose added a commit that referenced this issue Aug 29, 2024
@madalynrose
fix: Can't toggle off from the ad-hoc tools menu if you've toggled on…
9c0dafa 
… in DetailsView (#7431)

#### 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
`CardSelectionActionCreator`s
* 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

<!-- Are there any parts that you've intentionally left out-of-scope for
a later PR to handle? -->

<!-- Were there any alternative approaches you considered? What
tradeoffs did you consider? -->

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [x] Addresses an existing issue: #6253
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report
at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic
tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See
`CONTRIBUTING.md`.

---------

Co-authored-by: Madalyn Parker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
Accessibility Insights
Status: Needs release
Milestone
No milestone
Development

No branches or pull requests
7 participants
@codeofdusk @madalynrose @ferBonnin @sfoslund @JGibson2019 @DaveTryon @lisli1