fix: Create reflowable card footer buttons #5852
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pownkel
changed the title
9 tasks
madalynrose
requested changes
Aug 9, 2022
There was a problem hiding this comment.
This looks awesome! What a great improvement!
The only thing I'm hesitant about is the name CardFooterFarButtons. What is the intent of the word Far here? I think we can come up with a more illustrative name.
Ideas:
CardFooterMenuItemskeeps naming aligned withCardFooterMenuItemsBuilderand brings to mind that this contains items that might be presented as a menu.CardFooterInstanceActionButtonscaptures that these are the actions a user can take on the failure instance (and also these areActionButtons)
|
Thanks for the suggestion, @madalynrose! I changed the class name to |
6 tasks
pownkel
added a commit
that referenced
this pull request
Aug 25, 2022
…ns (#5935) #### Details Use store state instead of component state to determine when the issue filing settings dialog is open ##### Motivation We should avoid using component state where possible, because it can lead to tightly coupled components if we decide to split up a file. In this case, we currently need to pass callbacks to CardFooterMenuItemsBuilder to set the state of CardFooterInstanceActionButtons, which is a confusing flow of data and doesn't fit the flux paradigm. Instead of a background store, this PR creates a view-side CardsViewStore with the same pattern as TabStopsViewStore, which will reset its state when the page is refreshed so the dialog doesn't stay open between refreshes. ##### Context Continues a change from #5852. CardFooterInstanceActionButtons is not currently used anywhere in the UI, but will be wired up in a future PR. We currently have another dialog, the Export Report dialog, which is controlled by component state instead of store state. We may want to refactor that in a future PR for consistency.
8 tasks
pownkel
added a commit
that referenced
this pull request
Sep 16, 2022
#### 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)   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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details
Extracts out some of the work in #5745, plus additional refactors:
The gifs below show the behavior or CardFooterFarButtons when it is resized, but this PR does not use CardFooterFarButtons anywhere in the UI. Replacing CardKebabMenuButton and passing NarrowModeStatus through to the cards will be done in a separate PR.
CardFooterFarButtons also makes sure that focus correctly returns to the "..." button or the "file issue" button when the issue filing settings dialog is closed:
Motivation
Partially addresses #5609
Context
Deferred to future PRs:
(copied from #5745) Also worth mentioning is that this PR uses the width of the entire screen to determine if cards should reflow, rather than the width of the cards themselves. This is not ideal, especially in electron where both the left nav and the right-hand screenshot also resize as the screen resizes, and the card width is affected by more than just screen width. It doesn't make a huge difference from a user perspective, but in the future, we may want to consider refactoring NarrowModeDetector to be more generic (it currently assumes that it's measuring the width of the whole screen and sets its thresholds accordingly) and adding another NarrowModeDetector to the element containing the cards.
Pull request checklist
yarn fastpassyarn test)<rootDir>/test-results/unit/coveragefix:,chore:,feat(feature-name):,refactor:). SeeCONTRIBUTING.md.