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

flanakin · 2024-10-29T08:33:17Z

src/web/components/SideBar.tsx

+    { name: 'PowerShell', icon: <FaCode />, route: '/powershell' },
+    { name: 'Bicep modules', icon: <FaDatabase />, route: '/bicep-registry' },
+    { name: 'Open data', icon: <FaCloud />, route: '/open-data' },
+    { name: 'Learning', icon: <FaUserGraduate />, external: true, route: 'https://learn.microsoft.com/en-us/cloud-computing/finops/toolkit/finops-toolkit-overview' }, // External link


Make sure you don't include locales ("en-us") in links. But in this case, can we create a dedicated page for this? We don't have one today, but we can copy the learning section of the home page as a starting point. It's okay if that happens in a separate PR.

Also, we should probably put learning resources first since you should learn about FinOps before you use all the tools that automate it 🤔 (I know the mockup had it last. Sorry about that.)

Suggested change

{ name: 'Learning', icon: <FaUserGraduate />, external: true, route: 'https://learn.microsoft.com/en-us/cloud-computing/finops/toolkit/finops-toolkit-overview' }, // External link

{ name: 'Learning', icon: <FaUserGraduate />, external: true, route: 'https://learn.microsoft.com/cloud-computing/finops/toolkit/finops-toolkit-overview' }, // External link

Okay, I can do that if it makes more sense. We probably need to brainstorm about this.

When you say put learning resources first do you mean the order of things on the sidebar? So under home comes learning, then the tools?

flanakin · 2024-10-29T08:38:10Z

src/web/components/TopMenuBar.tsx

+
+
+// Styled components for layout and customization
+const StyledCommandBar = styled.div`


I feel like putting CSS in TSX is an antipattern. What do others think?

This is what I started with when converting to CSS. These are styled components. I will convert to fluent ui if we get the same look and feel as a result. Then, we need to check if we can create global styles for components and call them out. I'm more than happy to change this and get input from others

Fluent uses Griffel and the makestyles is something I am using now for the finops home page. It is not here in this first pr, but we can have a working session to go through this and then I can adapt accordingly.

New commit contains only fluent v2 with makeStyles

src/web/package.json

flanakin · 2024-10-29T08:42:34Z

src/web/pages/PowerBIReportsPage.tsx

+
+export function PowerBIReportsPage() {
+    return (
+        <div style={{ display: 'flex', flexDirection: 'column', height: '100vh', overflowX: 'hidden', backgroundColor: '#f4f6f8' }}>


Why does this page have the TopMenuBar and SideBar explicitly called out? I didn't notice that in other pages.

It is an example of a page using the top menu bar and sidebar, the sample page has that too.

flanakin · 2024-10-29T08:43:56Z

src/web/pages/SamplePage.tsx

Do we need this file?

We don't it is just to be able to show the top menu bar and sidebar and get your comments in.

flanakin · 2024-10-29T08:45:59Z

src/web/public/logo-windows.png

Shouldn't we put all images in an 'img' or 'images' folder? I'm not too picky on the name, but I'm assuming we'll want them organized in some way.

What I would like us to do is to define the directory structure and define how we want the files to be organized. There is examples on the internet, but I'm interested in understanding how the internal teams organize this as well

flanakin · 2024-10-29T08:46:55Z

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

nit: sidebar-collapse.png
Does Fluent UI have an equivalent? Not a huge deal.

I didn't find it, but I'll look into it.

I exclusively used fluent in the last commit, just pushed it. Not the same as in the figma design, but I will go through the icons library and find an appropriate one, for now I used chevron.

After searching in the catalog of fluent v2, I found it. Will do a new commit soon.

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

src/web/tsconfig.json

flanakin · 2024-10-30T20:52:48Z

src/web/components/FluentUIProvider.tsx

@Ivanmtta Creating this issue as a blocker for your review. Can you resolve this when you submit your review?

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 October 30, 2024 16:28

flanakin reviewed Oct 30, 2024

View reviewed changes

flanakin added this to the Web app milestone Oct 31, 2024

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

flanakin Oct 29, 2024

jamelachahbar Oct 29, 2024

jamelachahbar Oct 30, 2024

flanakin Oct 29, 2024

jamelachahbar Oct 29, 2024

jamelachahbar Oct 29, 2024

jamelachahbar Oct 30, 2024

flanakin Oct 29, 2024

jamelachahbar Oct 29, 2024

flanakin Oct 29, 2024

jamelachahbar Oct 29, 2024

flanakin Oct 29, 2024

jamelachahbar Oct 29, 2024 •

edited

Loading

flanakin Oct 29, 2024

jamelachahbar Oct 29, 2024 •

edited

Loading

jamelachahbar Oct 30, 2024 •

edited

Loading

jamelachahbar Oct 31, 2024

flanakin Oct 30, 2024

	{ name: 'Learning', icon: <FaUserGraduate />, external: true, route: 'https://learn.microsoft.com/en-us/cloud-computing/finops/toolkit/finops-toolkit-overview' }, // External link
	{ name: 'Learning', icon: <FaUserGraduate />, external: true, route: 'https://learn.microsoft.com/cloud-computing/finops/toolkit/finops-toolkit-overview' }, // External link



		// Styled components for layout and customization
		const StyledCommandBar = styled.div`

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

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 Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamelachahbar Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

jamelachahbar Oct 30, 2024 • edited Loading

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`?

jamelachahbar Oct 29, 2024 •

edited

Loading

jamelachahbar Oct 29, 2024 •

edited

Loading

jamelachahbar Oct 30, 2024 •

edited

Loading