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

feat: investigate fluentui v9 migration #3286

Closed
wants to merge 15 commits into from
Closed

feat: investigate fluentui v9 migration #3286

wants to merge 15 commits into from

Conversation

Mnickii
Copy link
Contributor

@Mnickii Mnickii commented Aug 21, 2024 •
edited
Loading

Overview

This PR investigates possibility of migrating GE components to use fluentUI v9.
Migrates some button usage and investigates theming changes

W.I.P

Demo

Notes

Theming changes require more work
This PR showcases instances where some component counterparts may not work to the desired GE experience

Testing Instructions

  • How to test this PR : Test on the jolly link

@Mnickii Mnickii marked this pull request as ready for review August 21, 2024 09:06
@Mnickii Mnickii requested a review from a team as a code owner August 21, 2024 09:06
@Mnickii Mnickii force-pushed the poc/fluent-component-migration branch from da9489a to 9160c65 Compare August 21, 2024 09:17
@Mnickii Mnickii force-pushed the poc/fluent-component-migration branch 2 times, most recently from ab1576f to 1ca66ae Compare August 21, 2024 10:30
@Mnickii Mnickii marked this pull request as draft August 21, 2024 11:42
@github-actions GitHub Actions
Copy link
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-3286.centralus.azurestaticapps.net

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-3286.centralus.azurestaticapps.net

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@github-actions GitHub Actions
Copy link
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-3286.centralus.azurestaticapps.net

@adhiambovivian
Copy link

@Mnickii some feedback;

1.Please ensure the run query text area is responsive to the size of the Graph query. In this upgrade, the size of this text area is not responsive to the Graph query text string size

image

  1. The docs icon, sample query text and method names seems to be 'floating' ie too much padding. Can the size of these be increased to reduce the padding?
    image

  2. The dividers here are overlapping, refer to the image
    image

  3. Vertical scrollbars for Samples is missing - refer to the image (taken from Graph Explorer pro)
    image

  4. The header is not aligned with the rest of the GE components mainly the share icon. Additionally, the share icon (on my screen) has be cut off and the rest of the components on the right side of the screen. When Graph Explorer view is minimized using the minimize side bar (at the top left), the problem does not exist
    image

  5. Any reason why the docs icons changed from blue to black & white?
    image

  6. The GE resources, history etc icons also changed from blue to black and white. When you hover the icons, only the sample queries icon changes color to blue.
    image

  7. Consent type on the permission tab is not visible even when I resize the browser tab window or zoom out
    image

@Mnickii
Copy link
Contributor Author

Mnickii commented Aug 27, 2024

@adhiambovivian as this is still a work in progress and a proof of concept, not all parts of the application have been migrated into fluentUI v9. This is meant to showcase how migration can be done, and in particular, how some experiences may need adapting to use different components. Theming also hasn't been migrated app-wide. Some of these changes are fixable in the scope of the POC but others would require actual project-wide migration.

musale
musale reviewed Aug 27, 2024
View reviewed changes
config/webpack.config.js
@@ -419,6 +420,8 @@ module.exports = function (webpackEnv) {
new webpack.optimize.LimitChunkCountPlugin({
maxChunks: 1
}),
// remove after comparing bundle sizes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any insights on bundle sizes?

musale
musale reviewed Aug 27, 2024
View reviewed changes
src/app/views/Layout.styles.ts
@@ -0,0 +1,94 @@
import { makeStyles, tokens } from '@fluentui/react-components';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the new designs and the effort needed to make the component look as the design as possible, what is the effort you figure is needed to achieve the migration? Are there lots of CSS tinkering? Does this file also mean there will be a change in the way styling is done (theming)?

@musale
Copy link
Contributor

musale commented Oct 3, 2024

@Mnickii going forward, is it possible to break this out into smaller portions where we have scoped PRs?

@Mnickii
Copy link
Contributor Author

Mnickii commented Dec 11, 2024

Closing this: Investigation complete and work in progress, tracked by #3294

@Mnickii Mnickii closed this Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@musale musale musale left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mnickii @adhiambovivian @musale