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

[Usability] Reflow buttons in failure cards #5609

Closed
ferBonnin opened this issue Jun 9, 2022 · 6 comments
Closed

[Usability] Reflow buttons in failure cards #5609

ferBonnin opened this issue Jun 9, 2022 · 6 comments
Labels
bug Something isn't working status: resolved This issue has been merged into main and deployed to canary.

Comments

@ferBonnin
Copy link
Contributor

Describe the bug

currently the failure cards in the automated checks and needs review instances have a "..." button by default. When this UI was first implemented, we determined reflowing the buttons to file bugs and copy failure details was not feasible. This bug is to revisit this after we updated the fluent UI version used

To Reproduce
Steps to reproduce the behavior:

  1. Go to accessible university demo site
  2. Run FastPass > automated checks
  3. notice how the failure cards always have the "..." button

Expected behavior

the two buttons inside that menu appear / disappear as reflow allows

Context (please complete the following information)

  • OS Name & Version: N/A
  • AI-Web Version & Environment: Prod 2.32

Are you willing to submit a PR?

No

Did you search for similar existing issues?

Yes

Additional context

Usability. Exploratory bug; if we determine this is now feasible let's chat with PM
@ferBonnin ferBonnin added the bug Something isn't working label Jun 9, 2022
@ghost ghost added the status: new This issue is new and requires triage by DRI. label Jun 9, 2022
@madalynrose madalynrose 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 Jun 10, 2022
@ghost ghost assigned ferBonnin Jun 10, 2022
@ghost
Copy link

ghost commented Jun 10, 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 ferBonnin added the status: ready for work This issue is ready to be worked on. label Jun 13, 2022
@ghost ghost removed the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label Jun 13, 2022
@pownkel pownkel assigned pownkel and unassigned ferBonnin Jul 11, 2022
@pownkel pownkel added status: active This issue is currently being worked on by someone. and removed status: ready for work This issue is ready to be worked on. labels Jul 11, 2022
@pownkel
Copy link
Contributor

pownkel commented Jul 12, 2022 •
edited
Loading

@ferBonnin I don't have much context for why this was originally infeasible, but I have a working version on my own branch that reflows those buttons similarly to how it's done in the command bar and handles focus when the issue filing dialog closes:

gif showing the Automated Checks screen being expanded and the kebab button on cards expanding to show the "file issue" and "copy failure details" buttons
gif showing the issue filing settings dialog being opened and closed at different widths. When the dialog is closed, focus returns to either the kebab button or the "file issue" button, depending on what is visible

The code will probably need some cleaning up and testing, but this change seems feasible.

@pownkel pownkel added status: needs author feedback This issue requires additional information from the issue author. and removed status: active This issue is currently being worked on by someone. labels Jul 12, 2022
@ghost
Copy link

ghost commented Jul 12, 2022

The team requires additional author feedback; please review their replies and update this issue accordingly. Thank you for contributing to Accessibility Insights!

@pownkel pownkel assigned ferBonnin and unassigned pownkel Jul 12, 2022
@ferBonnin
Copy link
Contributor Author

This is awesome, thanks @pownkel! We should do this change, please!

@ghost ghost added status: needs attention The author has provided additional feedback and now requires attention. and removed status: needs author feedback This issue requires additional information from the issue author. labels Jul 13, 2022
@pownkel pownkel mentioned this issue Jul 14, 2022
Closed
9 tasks
@ferBonnin ferBonnin added the status: active This issue is currently being worked on by someone. label Jul 15, 2022
@ghost ghost removed the status: needs attention The author has provided additional feedback and now requires attention. label Jul 15, 2022
@pownkel pownkel mentioned this issue Jul 30, 2022
Merged
4 tasks
pull bot pushed a commit to RahmaNiftaliyev/accessibility-insights-web that referenced this issue Aug 17, 2022
@pownkel
fix: Create reflowable card footer buttons (microsoft#5852)
7c7f09c 
#### Details
Extracts out some of the work in microsoft#5745, plus additional refactors:
- Add a new narrowModeThreshold for reflowing card footers
- Create a new component, CardFooterFarButtons, a reflowable and refactored version of CardKebabMenuButton

##### Motivation
Partially addresses microsoft#5609

##### Context
Deferred to future PRs:
- Further refactor CardFooterFarButtons to use store state instead of UI state to determine whether the issue filing dialog is open (this will allow renderIssueFilingSettingContent() to be extracted to a different file)
- Replace CardKebabMenuButton with CardFooterFarButtons in the cards UI
- Pass NarrowModeStatus through to the cards components
@pownkel pownkel mentioned this issue Aug 25, 2022
Merged
8 tasks
pownkel added a commit that referenced this issue Sep 16, 2022
@pownkel
fix: replace card footer component with reflowable version (#5966)
a353a2e 
#### Details

Replaces `CardKebabMenuButton` with CardFooterInstanceActionButtons`,
which will reflow the buttons currently in the kebab menu in the card
footer in both web and electron. GIFs below showing the card footer
reflow in action (copied from #5852)
![card footer buttons reflowing as window is
resized](https://user-images.githubusercontent.com/55459788/178857149-63590123-322d-4243-a4c1-cbcbe876ab9d.gif)
![demo of focus handling when issue filing settings is opened and
closed](https://user-images.githubusercontent.com/55459788/178857155-8de62725-cef2-4937-a9e1-b54e17747c79.gif)

Other changes in this PR:
- Initialize a CardsViewStore and relevant objects in DetailsView and
render-initializer
- Move rendering for the issue filing settings dialogue into a
higher-level component (`IssuesTable` in web and `TestView` in Electron)
to fix a bug where every card was rendering its own separate dialogue,
each with callbacks to focus a different button
- Add state relevant to the issue filing settings dialogue to
`CardsViewStoreData`

##### Motivation

Finishes addressing #5609 to add reflow behavior to the cards UI

##### Context

Continues work from #5852 and #5935

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [x] Addresses an existing issue: #5609
- [x] Ran `yarn null:autoadd`
- [x] Ran `yarn fastpass`
- [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`.
- [x] (UI changes only) Added screenshots/GIFs to description above
- [x] (UI changes only) Verified usability with NVDA/JAWS
@pownkel pownkel removed the status: active This issue is currently being worked on by someone. label Sep 19, 2022
@pownkel pownkel added the status: resolved This issue has been merged into main and deployed to canary. label Sep 19, 2022
@pownkel
Copy link
Contributor

pownkel commented Sep 19, 2022

@ferBonnin this is complete and the change is in canary!

@ferBonnin
Copy link
Contributor Author

thanks @pownkel, looks great!

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 status: resolved This issue has been merged into main and deployed to canary.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@madalynrose @ferBonnin @pownkel