Skip to content

Commit

Permalink
feat(fluentUI-migration): ActionButton fluentui v8 to v9 migration (#…
Browse files Browse the repository at this point in the history
…7401)

#### Details

Action Button migration from V8 to V9
| V8 Component | V9 Component | Reference Link |
|--------|--------|--------|
| Action Button | Button |
https://react.fluentui.dev/?path=/docs/components-button-button--docs |
| Primary Button | Button |
https://react.fluentui.dev/?path=/docs/components-button-button--docs |
| CommandBarButton | Combination of Menu, MenuTrigger, MenuButton,
MenuPopover, MenuList | 1. MenuButton -
https://react.fluentui.dev/?path=/docs/components-button-menubutton--docs
2. Menu -
https://react.fluentui.dev/?path=/docs/components-menu-menu--docs 3.
MenuList -
https://react.fluentui.dev/?path=/docs/components-menu-menulist--docs |
| IconProps for Action Button and action button internally takes care of
showing the icon | Created a component
**src/common/icons/fluentui-v9-icons.tsx** to consume
@fluentui/react-icons icons | Cell |

### Styling Improvements:

* Updated the width property from `100%` to `auto` for better layout
flexibility in multiple snapshot files.
* Added new CSS classes for better button and icon styling, including
hover and focus states.
* Adjusted font-weight to `normal` for better text consistency.

### Component Enhancements:

* Added `FluentUIV9Icon` import and updated the components to use the
new icon.
* Improved the components with new styles and updated its component
structure to use Fluent UI v9 components.

##### Motivation

1. Migrate Action Button of V8 to V9 Button in all locations of code. 
2. Changed types to align with V9 button.
3. Changed styling to align with old UI.
4. Fixed test cases to cover >90% coverage.


##### Context

UI locations for all the files are mentioned with images in below file:


[ActionButtonComparison](https://microsoft-my.sharepoint.com/:w:/r/personal/v-singhanjal_microsoft_com/_layouts/15/Doc.aspx?sourcedoc=%7B8893E880-1CFB-4D25-8E5F-3AD94CA0E9EB%7D&file=Document%202.docx&action=editNew&mobileredirect=true&wdOrigin=MARKETING.WORD.SIGNIN%2CAPPHOME-WEB.BANNER.NEWBLANK&wdPreviousSession=2dab2adc-a3f9-4816-a2b9-7175f764cb0f&wdPreviousSessionSrc=AppHomeWeb&ct=1724306297464&share=IQGA6JOI-xwlTY5fOtlMoOnrAX4RP9xcmRvEolaQPAyCsOs)


Due to FluentUI V9 Migration there are slight differences between old
and new Icons. PFA screenshots below.

- **More Actions, Collapse all Icons**


![image](https://github.com/user-attachments/assets/db29f19c-b089-4f7f-996c-ccac195ee0a6)

![image](https://github.com/user-attachments/assets/dcf1041f-a1b1-4a81-b88a-65643fc805c4)

- **Expand, Move to Assessment Icons**


![image](https://github.com/user-attachments/assets/1637ab2c-874f-47a1-a702-f71f82a9d920)

![image](https://github.com/user-attachments/assets/69e55a22-b464-441d-b22d-e6ca617ed6fe)

- **Export result, Save Assessment, Load Assessment, Start over Icons**


![image](https://github.com/user-attachments/assets/c34995c5-4b1d-46bc-8bb8-b4631f7065c2)

![image](https://github.com/user-attachments/assets/331cf997-531b-475a-a2ac-31551d50e60b)

- **File issue, Copy failure details Icons**


![image](https://github.com/user-attachments/assets/ddc57a1c-e8cd-4c25-b798-a948c80f61ed)

![image](https://github.com/user-attachments/assets/aacfd3df-dc58-4df5-8380-76a41cad190e)






Note : 

1. isNarrowMode property is added in details-view-command-bar.tsx and
other related files to differentiate the rendering of button in normal
mode and menu in minimize mode.
2. SaveAssessmentDialog is now moved out of SaveAssessmentButton file as
with v9 button migration to menu, dialog in narrow mode was not
rendering as it was getting close once menu button is clicked.
3. Accessibility test has a failure for aria-hidden-focus. There is an
existing issue with fluentui and as per them these are Tabster dummy
inputs and they must be added to the exception list. They are
intentionally aria - hidden as their purpose is to redirect focus to the
correct element right when they receive focus and not having aria -
hidden will result in the screen readers choking starting to announce
them.
Please refer for more information on the
error:microsoft/fluentui#25133
5. Changed buttonRef: IRefObject<IButton>; to ButtonRefFunction, as
ButtonRefFunction can be used in future when we migrate the dialog to v9
from V8.
6. Changed /load-assessment-button.tsx to functional component from
class component so that i can consume makeStyles hooks, as hooks cannot
be used in class components, hence converted to Functional component.
7. Removed IPoint because its deprecated in latest Fluent UI V8.
8. Removed expand-collapse-all-button.scss because to consume makeStyles
hooks and to use V9 themes, hence created
expand-collapse-all-button.-styles.tsx.

Technical Debt:
These will be take care in up coming PR for v8 to v9 migration.
**IButton**:
1. We are stilling using IButton from v8 for ref object in
src\DetailsView\components\details-view-command-bar.tsx, because when we
tried to use useRef instead of IButton, we have converted the component
from class to functional component to consume useRef hook.
2. But after converting to functional component and using useRef, focus
back to kebab button is not going in narrow mode when we close the
dialog.

**Card footer button scroll behaviour**
1. If multiple issues are shown in the card footer, when we expand all
issues, and if we click on the first issue section button, scroll goes
to the bottom of the page i.e, the focus goes to the last section.

**Closing dialog focus issue**
1. In Narromode,When we open the dialog on click of export or save
assessment or start over, and when we close the dialog, focus is not
going back to the Kebab button, as dialog is still in v8, we will take
care of this focus back issues during v9 dialog migration.

**Focus style inconsistency**
1. We explicitly added focus styling for buttons to show focus like
kebab button / buttons, after closing the file issue dialog, but when we
use keyboard to navigate to these kebab or other buttons, browser
default focus styles are getting applied, for which it might look thick
border for keyboard focus, but on close of dialog, the focus goes to
button with little thin border.

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [ ] Addresses an existing issue: #0000
- [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`
- [ ] 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

---------

Co-authored-by: v-sharmachir <[email protected]>
Co-authored-by: Prachi Naigaonkar <[email protected]>
Co-authored-by: Chirag Sharma <[email protected]>
Co-authored-by: Jeevani Chinthala <[email protected]>
Co-authored-by: Anjali Singh <[email protected]>
  • Loading branch information
6 people committed Sep 19, 2024
1 parent 5aba30e commit 15a87c1
Show file tree
Hide file tree
Showing 102 changed files with 4,084 additions and 1,867 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@
border: none;
display: flex;
align-items: baseline;
width: 100%;
width: auto;
}
.collapsible-container .collapsible-control:hover {
background-color: var(--neutral-alpha-4);
Expand Down Expand Up @@ -993,13 +993,27 @@
color: buttontext !important;
}
}
.kebab-menu-icon--EUIj-:hover {
border-style: solid !important;
border-width: 1px;
padding-left: 8px !important;
padding-right: 8px !important;
padding-top: 5px !important;
padding-bottom: 5px !important;
.fileissue--x5mek {
font-weight: 400 !important;
}
.fileissue--x5mek:hover > span > svg {
color: var(--insights-button-hover);
}
.fileissue--x5mek:focus {
outline: 1px solid;
}
.copyfailuredetails--NHZE4 {
font-weight: 400 !important;
}
.copyfailuredetails--NHZE4:hover > span > svg {
color: var(--insights-button-hover);
}
.menu-Button--ElHFS:hover {
border: 1px solid var(--primary-text) !important;
color: var(--primary-text) !important;
}
.menu-Button--ElHFS:focus {
outline: 1px solid;
}
.kebab-menu-button--9Qt0a {
margin-right: 8px;
Expand Down Expand Up @@ -1105,6 +1119,7 @@
display: flex;
align-items: baseline;
text-align: left;
font-weight: normal;
}
.rule-details-group--Tb-LW .rule-detail .outcome-chip {
vertical-align: middle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@
border: none;
display: flex;
align-items: baseline;
width: 100%;
width: auto;
}
.collapsible-container .collapsible-control:hover {
background-color: var(--neutral-alpha-4);
Expand Down Expand Up @@ -993,13 +993,27 @@
color: buttontext !important;
}
}
.kebab-menu-icon--EUIj-:hover {
border-style: solid !important;
border-width: 1px;
padding-left: 8px !important;
padding-right: 8px !important;
padding-top: 5px !important;
padding-bottom: 5px !important;
.fileissue--x5mek {
font-weight: 400 !important;
}
.fileissue--x5mek:hover > span > svg {
color: var(--insights-button-hover);
}
.fileissue--x5mek:focus {
outline: 1px solid;
}
.copyfailuredetails--NHZE4 {
font-weight: 400 !important;
}
.copyfailuredetails--NHZE4:hover > span > svg {
color: var(--insights-button-hover);
}
.menu-Button--ElHFS:hover {
border: 1px solid var(--primary-text) !important;
color: var(--primary-text) !important;
}
.menu-Button--ElHFS:focus {
outline: 1px solid;
}
.kebab-menu-button--9Qt0a {
margin-right: 8px;
Expand Down Expand Up @@ -1105,6 +1119,7 @@
display: flex;
align-items: baseline;
text-align: left;
font-weight: normal;
}
.rule-details-group--Tb-LW .rule-detail .outcome-chip {
vertical-align: middle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@
border: none;
display: flex;
align-items: baseline;
width: 100%;
width: auto;
}
.collapsible-container .collapsible-control:hover {
background-color: var(--neutral-alpha-4);
Expand Down Expand Up @@ -993,13 +993,27 @@
color: buttontext !important;
}
}
.kebab-menu-icon--EUIj-:hover {
border-style: solid !important;
border-width: 1px;
padding-left: 8px !important;
padding-right: 8px !important;
padding-top: 5px !important;
padding-bottom: 5px !important;
.fileissue--x5mek {
font-weight: 400 !important;
}
.fileissue--x5mek:hover > span > svg {
color: var(--insights-button-hover);
}
.fileissue--x5mek:focus {
outline: 1px solid;
}
.copyfailuredetails--NHZE4 {
font-weight: 400 !important;
}
.copyfailuredetails--NHZE4:hover > span > svg {
color: var(--insights-button-hover);
}
.menu-Button--ElHFS:hover {
border: 1px solid var(--primary-text) !important;
color: var(--primary-text) !important;
}
.menu-Button--ElHFS:focus {
outline: 1px solid;
}
.kebab-menu-button--9Qt0a {
margin-right: 8px;
Expand Down Expand Up @@ -1105,6 +1119,7 @@
display: flex;
align-items: baseline;
text-align: left;
font-weight: normal;
}
.rule-details-group--Tb-LW .rule-detail .outcome-chip {
vertical-align: middle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@
border: none;
display: flex;
align-items: baseline;
width: 100%;
width: auto;
}
.collapsible-container .collapsible-control:hover {
background-color: var(--neutral-alpha-4);
Expand Down Expand Up @@ -993,13 +993,27 @@
color: buttontext !important;
}
}
.kebab-menu-icon--EUIj-:hover {
border-style: solid !important;
border-width: 1px;
padding-left: 8px !important;
padding-right: 8px !important;
padding-top: 5px !important;
padding-bottom: 5px !important;
.fileissue--x5mek {
font-weight: 400 !important;
}
.fileissue--x5mek:hover > span > svg {
color: var(--insights-button-hover);
}
.fileissue--x5mek:focus {
outline: 1px solid;
}
.copyfailuredetails--NHZE4 {
font-weight: 400 !important;
}
.copyfailuredetails--NHZE4:hover > span > svg {
color: var(--insights-button-hover);
}
.menu-Button--ElHFS:hover {
border: 1px solid var(--primary-text) !important;
color: var(--primary-text) !important;
}
.menu-Button--ElHFS:focus {
outline: 1px solid;
}
.kebab-menu-button--9Qt0a {
margin-right: 8px;
Expand Down Expand Up @@ -1105,6 +1119,7 @@
display: flex;
align-items: baseline;
text-align: left;
font-weight: normal;
}
.rule-details-group--Tb-LW .rule-detail .outcome-chip {
vertical-align: middle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@
border: none;
display: flex;
align-items: baseline;
width: 100%;
width: auto;
}
.collapsible-container .collapsible-control:hover {
background-color: var(--neutral-alpha-4);
Expand Down Expand Up @@ -993,13 +993,27 @@
color: buttontext !important;
}
}
.kebab-menu-icon--EUIj-:hover {
border-style: solid !important;
border-width: 1px;
padding-left: 8px !important;
padding-right: 8px !important;
padding-top: 5px !important;
padding-bottom: 5px !important;
.fileissue--x5mek {
font-weight: 400 !important;
}
.fileissue--x5mek:hover > span > svg {
color: var(--insights-button-hover);
}
.fileissue--x5mek:focus {
outline: 1px solid;
}
.copyfailuredetails--NHZE4 {
font-weight: 400 !important;
}
.copyfailuredetails--NHZE4:hover > span > svg {
color: var(--insights-button-hover);
}
.menu-Button--ElHFS:hover {
border: 1px solid var(--primary-text) !important;
color: var(--primary-text) !important;
}
.menu-Button--ElHFS:focus {
outline: 1px solid;
}
.kebab-menu-button--9Qt0a {
margin-right: 8px;
Expand Down Expand Up @@ -1105,6 +1119,7 @@
display: flex;
align-items: baseline;
text-align: left;
font-weight: normal;
}
.rule-details-group--Tb-LW .rule-detail .outcome-chip {
vertical-align: middle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@
border: none;
display: flex;
align-items: baseline;
width: 100%;
width: auto;
}
.collapsible-container .collapsible-control:hover {
background-color: var(--neutral-alpha-4);
Expand Down Expand Up @@ -993,13 +993,27 @@
color: buttontext !important;
}
}
.kebab-menu-icon--EUIj-:hover {
border-style: solid !important;
border-width: 1px;
padding-left: 8px !important;
padding-right: 8px !important;
padding-top: 5px !important;
padding-bottom: 5px !important;
.fileissue--x5mek {
font-weight: 400 !important;
}
.fileissue--x5mek:hover > span > svg {
color: var(--insights-button-hover);
}
.fileissue--x5mek:focus {
outline: 1px solid;
}
.copyfailuredetails--NHZE4 {
font-weight: 400 !important;
}
.copyfailuredetails--NHZE4:hover > span > svg {
color: var(--insights-button-hover);
}
.menu-Button--ElHFS:hover {
border: 1px solid var(--primary-text) !important;
color: var(--primary-text) !important;
}
.menu-Button--ElHFS:focus {
outline: 1px solid;
}
.kebab-menu-button--9Qt0a {
margin-right: 8px;
Expand Down Expand Up @@ -1105,6 +1119,7 @@
display: flex;
align-items: baseline;
text-align: left;
font-weight: normal;
}
.rule-details-group--Tb-LW .rule-detail .outcome-chip {
vertical-align: middle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@
border: none;
display: flex;
align-items: baseline;
width: 100%;
width: auto;
}
.collapsible-container .collapsible-control:hover {
background-color: var(--neutral-alpha-4);
Expand Down Expand Up @@ -993,13 +993,27 @@
color: buttontext !important;
}
}
.kebab-menu-icon--EUIj-:hover {
border-style: solid !important;
border-width: 1px;
padding-left: 8px !important;
padding-right: 8px !important;
padding-top: 5px !important;
padding-bottom: 5px !important;
.fileissue--x5mek {
font-weight: 400 !important;
}
.fileissue--x5mek:hover > span > svg {
color: var(--insights-button-hover);
}
.fileissue--x5mek:focus {
outline: 1px solid;
}
.copyfailuredetails--NHZE4 {
font-weight: 400 !important;
}
.copyfailuredetails--NHZE4:hover > span > svg {
color: var(--insights-button-hover);
}
.menu-Button--ElHFS:hover {
border: 1px solid var(--primary-text) !important;
color: var(--primary-text) !important;
}
.menu-Button--ElHFS:focus {
outline: 1px solid;
}
.kebab-menu-button--9Qt0a {
margin-right: 8px;
Expand Down Expand Up @@ -1105,6 +1119,7 @@
display: flex;
align-items: baseline;
text-align: left;
font-weight: normal;
}
.rule-details-group--Tb-LW .rule-detail .outcome-chip {
vertical-align: middle;
Expand Down
5 changes: 4 additions & 1 deletion src/DetailsView/components/assessment-instance-table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { IRenderFunction } from '@fluentui/utilities';
import { AssessmentDefaultMessageGenerator } from 'assessments/assessment-default-message-generator';
import { InstanceTableHeaderType, InstanceTableRow } from 'assessments/types/instance-table-data';
import { InsightsCommandButton } from 'common/components/controls/insights-command-button';
import { FluentUIV9Icon } from 'common/icons/fluentui-v9-icons';
import { ManualTestStatus } from 'common/types/store-data/manual-test-status';
import { hasIn } from 'lodash';
import * as React from 'react';
Expand Down Expand Up @@ -125,7 +126,9 @@ export class AssessmentInstanceTable extends React.Component<AssessmentInstanceT
return (
<InsightsCommandButton
data-automation-id={passUnmarkedInstancesButtonAutomationId}
iconProps={{ iconName: 'skypeCheck' }}
insightsCommandButtonIconProps={{
icon: <FluentUIV9Icon iconName="Checkmark20Filled" />,
}}
onClick={this.onPassUnmarkedInstances}
disabled={disabled}
>
Expand Down
Loading

0 comments on commit 15a87c1

Please sign in to comment.