PR for [UX] Migrate existing markdown content to TSX pages #1056 Turn UX Design to Implementation #1077

jamelachahbar · 2024-10-27T20:01:47Z

🛠️ Description

#1056
PR for turning figma design into implementation.
Top Menu Bar, Sidebar + Sample Page created.
Tests created

📷 Screenshots

Jest test results

📋 Checklist

🔬 How did you test this change?

🤏 Lint tests

[] 🤞 PS -WhatIf / az validate

👍 Manually deployed + verified

💪 Unit tests

🙌 Integration tests

🙋‍♀️ Do any of the following that apply?

🚨 This is a breaking change.

🤏 The change is less than 20 lines of code.

📑 Did you update `docs/changelog.md`?

✅ Updated changelog (required for dev PRs)

➡️ Will add log in a future PR (feature branch PRs only)

❎ Log not needed (small/internal change)

📖 Did you update documentation?

✅ Public docs in docs (required for dev)

✅ Internal dev docs in src (required for dev)

➡️ Will add docs in a future PR (feature branch PRs only)

❎ Docs not needed (small/internal change)

…web app for marketing site and management portal (#1024)

…olkit into jachahbar/ux

…tead of React.FC

…s and assets

jamelachahbar · 2024-10-27T20:07:08Z

@flanakin, I forget to sync with latest from ux branch. There are conflicts that need to be resolved.
Should we always create new branch with each change? This one contains the commits of the previews pr, only the last commit should be reviewed.

src/web/pages/App.tsx

jamelachahbar · 2024-10-27T23:21:58Z

Thanks, I forgot it. I'll commit it

…

On Sun, 27 Oct 2024 at 23:25, Orthodoxos Kipouridis < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/web/pages/App.tsx <#1077 (comment)> : > @@ -1,27 +1,41 @@ -import './App.css'; +import { BrowserRouter as Router, Route, Routes } from 'react-router-dom'; +import HomePage from './HomePage'; +import FinOpsHubsPage from './FinOpsHubsPage'; +import FinOpsWorkbooksPage from './FinOpsWorkbooksPage'; +import GovernanceWorkbookPage from './GovernanceWorkbookPage'; +import AzureOptimizationEnginePage from './AzureOptimizationEnginePage'; +import PowerShellModulePage from './PowerShellModulePage'; +import BicepRegistryModulesPage from './BicepRegistryModulesPage'; +import OpenDataPage from './OpenDataPage'; +import {PowerBIReportsPage} from './PowerBIReportsPage'; +import CostOptimizationWorkbookPage from './CostOptimizationWorkbookPage'; +import {SamplePage} from './SamplePage'; +import FluentUIProvider from '../components/FluentUIProvider'; // Use your FluentUIProvider Can it be that the FluentUIProvider in the components folder was not committed? Its not in this path — Reply to this email directly, view it on GitHub <#1077 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A6QJFYCJ4GI4KTGWUHUYJZTZ5VK3LAVCNFSM6AAAAABQWD2W6SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOJXGYZTSNRUGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

… pages

akiskips

Tested deployment locally

src/web/components/SideBar.tsx

src/web/components/TopMenuBar.tsx

src/web/package.json

src/web/pages/PowerBIReportsPage.tsx

src/web/pages/SamplePage.tsx

src/web/public/logo-windows.png

src/web/public/sidebar-left-svgrepo-com.png

src/web/public/sidebar-right-svgrepo-com.png

src/web/tsconfig.json

This reverts commit 4a62a9b. Using fluent icons24 and improve sidebar design

… test is successful

allcontributors · 2024-11-06T16:49:01Z

@microsoft-github-policy-service[bot]

@jamelachahbar already contributed before to code

Ivanmtta · 2024-11-08T07:02:16Z

src/web/components/FluentUIProvider.tsx

+import { FluentProvider, webLightTheme } from '@fluentui/react-components';
+
+interface Props {
+  children: React.ReactNode;


Import ReactNode directly

Ivanmtta · 2024-11-08T07:02:24Z

src/web/components/FluentUIProvider.tsx

+import React from 'react';
+import { FluentProvider, webLightTheme } from '@fluentui/react-components';
+
+interface Props {


Ivanmtta · 2024-11-08T07:02:45Z

src/web/components/FluentUIProvider.tsx

+  children: React.ReactNode;
+}
+
+function FluentUIProvider({ children }: Props) {


Lets use arrow functions for functional components

Also, Add JSDOC

Ivanmtta · 2024-11-08T07:03:31Z

src/web/components/SideBar.tsx

+  People24Filled,
+  PanelLeftExpandRegular,
+  PanelLeftContractRegular,
+} from "@fluentui/react-icons";


Single quotes for TS code

Ivanmtta · 2024-11-08T07:03:43Z

src/web/components/SideBar.tsx

+  PanelLeftContractRegular,
+} from "@fluentui/react-icons";
+
+// Styles for the sidebar using Fluent UI's `makeStyles`


No need for this comment

Ivanmtta · 2024-11-08T07:05:03Z

src/web/components/SideBar.tsx

+} from "@fluentui/react-icons";
+
+// Styles for the sidebar using Fluent UI's `makeStyles`
+const useStyles = makeStyles({


Lets move styling to a separate CSS file, that way it will not have to be compiled.

It will also be easier to write since we will have IntelliSense

Ivanmtta · 2024-11-08T07:05:51Z

src/web/components/SideBar.tsx

+// Styles for the sidebar using Fluent UI's `makeStyles`
+const useStyles = makeStyles({
+    sidebar: {
+      minHeight: '100vh',


Ivanmtta · 2024-11-08T07:06:08Z

src/web/components/SideBar.tsx

+const useStyles = makeStyles({
+    sidebar: {
+      minHeight: '100vh',
+      backgroundColor: '#f4f6f8',


How does this affect dark/light theme?

Ivanmtta · 2024-11-08T07:06:25Z

src/web/components/SideBar.tsx

+      position: 'relative',
+      transition: 'width 0.2s ease',
+      paddingTop: '40px',
+      paddingRight: '18px',


Can we use constants for these?

Ivanmtta · 2024-11-08T07:07:04Z

src/web/components/SideBar.tsx

+
+// Styles for the sidebar using Fluent UI's `makeStyles`
+const useStyles = makeStyles({
+    sidebar: {


Can we follow a naming for the classNames? that way they dont collide with Fluent's. Something like ftk-componentName-sidebar

Ivanmtta · 2024-11-08T07:08:25Z

src/web/components/FluentUIProvider.tsx

@@ -0,0 +1,16 @@
+import React from 'react';


Do we really need this component? could we add this provider in a root component?

Ivanmtta · 2024-11-08T07:09:02Z

src/web/components/SideBar.tsx

+      position: 'relative',
+      transition: 'background-color 0.2s ease, color 0.2s ease',
+      '&:hover': {
+        backgroundColor: '#E6E6E6', // Updated neutral grey background on hover


Lets not add comments for CSS, most are self explanitory, also colors should be variables. Also lets think of theming early

Ivanmtta · 2024-11-08T07:09:28Z

src/web/components/SideBar.tsx

+
+const menuItems = [
+  { name: 'Home', icon: <Home24Regular />, filledIcon: <Home24Filled />, route: '/' },
+  { name: 'FinOps Hubs', icon: <DataUsage24Regular />, filledIcon: <DataUsage24Filled />, route: '/hubs' },


Can we move the routes to an enum?

Ivanmtta · 2024-11-08T07:09:57Z

src/web/components/SideBar.tsx

+  { name: 'Contributors', icon: <People24Regular />, filledIcon: <People24Filled />, route: '/contributors' },
+];
+
+function SideBar() {


Arrow function and JSDOC

Ivanmtta · 2024-11-08T07:11:05Z

src/web/components/SideBar.tsx

+          icon={isCollapsed ? <PanelLeftExpandRegular /> : <PanelLeftContractRegular />}
+          className={classes.toggleButton}
+          onClick={toggleSidebar}
+          aria-label={isCollapsed ? 'Expand Sidebar' : 'Collapse Sidebar'}


Can we start early extracting all strings to a json file when we start thinking of our localization story?

Ivanmtta · 2024-11-08T07:11:30Z

src/web/components/TopMenuBar.tsx

@@ -0,0 +1,69 @@
+import { Text, makeStyles, Image, Divider } from '@fluentui/react-components';
+
+const useStyles = makeStyles({


Same comments as above

Ivanmtta · 2024-11-08T07:11:59Z

src/web/pages/App.tsx

+import OpenDataPage from './OpenDataPage';
+import {PowerBIReportsPage} from './PowerBIReportsPage';
+import CostOptimizationWorkbookPage from './CostOptimizationWorkbookPage';
+import {SamplePage} from './SamplePage';


{ SamplePage }

Ivanmtta · 2024-11-08T07:12:16Z

src/web/pages/App.tsx

+import {PowerBIReportsPage} from './PowerBIReportsPage';
+import CostOptimizationWorkbookPage from './CostOptimizationWorkbookPage';
+import {SamplePage} from './SamplePage';
+import FluentUIProvider from '../components/FluentUIProvider'; // Use your FluentUIProvider


remove comment

Ivanmtta · 2024-11-08T07:12:27Z

src/web/pages/App.tsx


+/**


use JSDOC style for comments

Ivanmtta · 2024-11-08T07:13:08Z

src/web/pages/AzureOptimizationEnginePage.tsx

+function OptimizationEnginePage() {
+  return (
+    <div>
+      <h1>Azure Optimization Engine</h1>


Can we start extracting all strings to a json file so we can later be able to handle localization?

jamelachahbar added 10 commits October 13, 2024 02:08

added scaffolding for homepage and tools + test for homepage

406c63d

[UX] Adapted name of LandingPage + improved Testing of HomePage - UX …

85730ba

…web app for marketing site and management portal (#1024)

removed unnecessary imports

de8b722

add page

b840c97

adapt code to have the same style as the other pages

9d44c77

Merge branch 'jachahbar/ux' of https://github.com/microsoft/finops-to…

9dac236

…olkit into jachahbar/ux

convert all pages and App.tsx to use a regular function component ins…

dae2910

…tead of React.FC

changed order of pages and linknames

5056164

removed trailing comma

e6f83fd

Topmenubar, sidebar and samplepage rendered based on ux design + test…

9ee7490

…s and assets

jamelachahbar requested review from flanakin and akiskips October 27, 2024 20:01

microsoft-github-policy-service bot requested a review from arthurclares October 27, 2024 20:02

microsoft-github-policy-service bot added the Needs: Review 👀 PR that is ready to be reviewed label Oct 27, 2024

microsoft-github-policy-service bot assigned arthurclares and flanakin Oct 27, 2024

akiskips reviewed Oct 27, 2024

View reviewed changes

src/web/pages/App.tsx Show resolved Hide resolved

jamelachahbar added 4 commits October 28, 2024 00:23

fluent provider added

e3dc28b

Updated test to work with samplepage and included test for finopshubs…

eb5038f

… pages

Add SideBar component

d1c001c

Adapted casing of SideBar

39ce5cb

akiskips self-requested a review October 28, 2024 12:04

akiskips approved these changes Oct 28, 2024

View reviewed changes

flanakin requested a review from Ivanmtta October 29, 2024 08:19

flanakin approved these changes Oct 29, 2024

View reviewed changes

jamelachahbar added 3 commits October 30, 2024 17:20

refactored to only use fluent v2

bd7e200

removed react icons package

583e573

removed spaces

b5a9719

microsoft-github-policy-service bot requested a review from flanakin November 1, 2024 15:40

jamelachahbar added 4 commits November 1, 2024 16:49

improve sidebar ui

7aa0478

Revert "ui enhancements using fluentui components and makeStyles"

d0bd651

This reverts commit 4a62a9b. Using fluent icons24 and improve sidebar design

removed fluentui provider as it is already present in App.tsx

91beff6

updated test suite and added aria label to button component to ensure…

93b5b3d

… test is successful

jamelachahbar closed this Nov 6, 2024

jamelachahbar reopened this Nov 6, 2024

jamelachahbar added 2 commits November 6, 2024 17:56

resolving conflict

895323a

removing additional styling and top and sidebarmenu

91a49cb

Ivanmtta reviewed Nov 8, 2024

View reviewed changes

src/web/pages/App.tsx

/**

Copy link

Member

Ivanmtta Nov 8, 2024

Choose a reason for hiding this comment

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

use JSDOC style for comments

Ivanmtta reviewed Nov 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR for [UX] Migrate existing markdown content to TSX pages #1056 Turn UX Design to Implementation #1077

PR for [UX] Migrate existing markdown content to TSX pages #1056 Turn UX Design to Implementation #1077

jamelachahbar commented Oct 27, 2024 •

edited

Loading

jamelachahbar commented Oct 27, 2024

jamelachahbar commented Oct 27, 2024 via email

akiskips left a comment

allcontributors bot commented Nov 6, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

Ivanmtta Nov 8, 2024

		@@ -0,0 +1,69 @@
		import { Text, makeStyles, Image, Divider } from '@fluentui/react-components';

		const useStyles = makeStyles({

PR for [UX] Migrate existing markdown content to TSX pages #1056 Turn UX Design to Implementation #1077

Are you sure you want to change the base?

PR for [UX] Migrate existing markdown content to TSX pages #1056 Turn UX Design to Implementation #1077

Conversation

jamelachahbar commented Oct 27, 2024 • edited Loading

🛠️ Description

📷 Screenshots

Jest test results

📋 Checklist

🔬 How did you test this change?

🙋‍♀️ Do any of the following that apply?

📑 Did you update docs/changelog.md?

📖 Did you update documentation?

jamelachahbar commented Oct 27, 2024

jamelachahbar commented Oct 27, 2024 via email

akiskips left a comment

Choose a reason for hiding this comment

allcontributors bot commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamelachahbar commented Oct 27, 2024 •

edited

Loading

📑 Did you update `docs/changelog.md`?