Feat/ToggleSwitch to TopBarMenu by kleinwave · Pull Request #208 · qua-platform/qualibrate-app

kleinwave · 2025-06-30T15:38:53Z

https://quantum-machines.atlassian.net/jira/software/c/projects/QUAL/boards/173?assignee=712020%3A2a0fc73a-d6d7-4126-9576-6e44837c6a01&selectedIssue=QUAL-1396

kleinwave · 2025-06-30T15:47:16Z

This PR does the following:

Refactored the ToggleSwitch that was used in JSONEditor.tsx to now be reused in the topbar menu
(this was the JSONEditor toggleswitch for reference!)
this is what's on the figma for reference
Added a new Refresh Icon over the old one in both the node library and graph library modules that functions just the same
The sidebar Menu now hides the graph status module from the upper part of menu and the icon itself changed to not have the green start symbol
Pressing "Run graph" redirects to the graph-library and pressing "Active graph" redirects to graph-status modules respectively

Smaller details:

TopbarMenu toggleswitch is only visible in the Graph library tab
Pressing the "Run" button on a graph redirects the page to the graph status module and automatically selects "Active Graph" on the toggle switch
Refreshing the page or going to another page in qualibrate and then returning still returns to whatever page they last left off on (graph-library if toggle switch was on "Run graph" or graph-status if was previously toggled to "Active graph")

manas-qm

There are some important things that must be done the right way, so please check the comments and implement changes acording to comments.

manas-qm · 2025-07-02T10:19:11Z

-  gap: 8px;
-  padding: 4px;
+  display: inline-flex;
+  border: 1px solid #444;


Add #444 color to the index.scss first!

manas-qm · 2025-07-02T10:19:59Z

+  left: 0;
+  width: 100%;
+  height: 100%;
+  background-color: #2ccbe599;


Add color to index.scss.

manas-qm · 2025-07-02T10:20:45Z

      {!toggleSwitch && <h1 style={{ paddingTop: "10px", paddingBottom: "5px" }}>{title}</h1>}
-      {toggleSwitch && <ToggleSwitch title={title} activeTab={activeTab} setActiveTab={setActiveTab} />}
+      {toggleSwitch && (
+        <div style={{ display: "flex", alignItems: "center", justifyContent: "space-between", paddingTop: "10px", paddingBottom: "5px" }}>


Please remove inline styles!

done
But looking at the rest of JSONEditor.tsx; inline styles are used everywhere. At some point we should refactor all of these into JSONEditor.module.scss (which btw wasn't even imported)

manas-qm · 2025-07-02T10:21:05Z

-      {toggleSwitch && <ToggleSwitch title={title} activeTab={activeTab} setActiveTab={setActiveTab} />}
+      {toggleSwitch && (
+        <div style={{ display: "flex", alignItems: "center", justifyContent: "space-between", paddingTop: "10px", paddingBottom: "5px" }}>
+          <h1 style={{ margin: 0 }}>{title}</h1>


Same thing here.

done - refer to comment above

manas-qm · 2025-07-02T10:24:00Z

    | undefined
  >(undefined);
  const [selectedPageName, setSelectedPageName] = useState<ModuleKey | null>(null);
+  const [graphTab, setGraphTab] = useState<string>("run");


We won't use this! It's not needed. We can use selectedPageName instead! So please remove graphTab and setGraphTab since FlexLayoutContext is global context wrapper so this is not a place to add some values that are needed for some specfic cases.

manas-qm · 2025-07-02T10:27:39Z

    path?: string;
    dataCy?: string;
  };
+  hidden?: boolean;


This isn't a good aproach. This page should have two tabs, not two separate pages where one of it is hidden. Please revert this and find a workaround.

manas-qm · 2025-07-02T10:28:49Z

+.refreshButton {
+  background: none;
+  border: none;
+  cursor: pointer;


Why background: none; border: none?
Are they needed?
Please check the WHOLE css added in this PR if it's needed!

Copilot

Pull Request Overview

This PR implements a toggle switch component in the top bar menu to allow switching between "Run graph" and "Active graph" views for graph-related pages. The changes consolidate graph navigation into a single sidebar menu item with dynamic behavior based on the toggle state.

Refactored ToggleSwitch component to be more flexible with configurable options
Added graph tab state management to FlexLayoutContext for coordinating between graph library and graph status views
Replaced text-based refresh buttons with icon-based buttons across multiple modules

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
frontend/src/ui-lib/Icons/RefreshIcon.tsx	New refresh icon component with button styling
frontend/src/common/ui-components/common/ToggleSwitch/ToggleSwitch.tsx	Refactored to accept configurable options instead of hardcoded Live/Final
frontend/src/routing/flexLayout/FlexLayoutContext.tsx	Added graphTab state management for coordinating graph views
frontend/src/modules/TopbarMenu/TitleBarMenu.tsx	Integrated toggle switch for graph pages with tab switching logic
frontend/src/modules/SidebarMenu/SidebarMenu.tsx	Modified graph menu behavior to respect toggle state
frontend/src/modules/GraphLibrary/index.tsx	Replaced text refresh button with icon button
frontend/src/modules/Nodes/index.tsx	Replaced text refresh button with icon button
frontend/src/routing/ModulesRegistry.ts	Hidden graph-status from sidebar and updated titles

Comments suppressed due to low confidence (3)

frontend/src/ui-lib/Icons/RefreshIcon.tsx:3

The component is named RefreshButton but the file is named RefreshIcon.tsx. The component name should be RefreshIcon to match the filename and export, or the file should be renamed to RefreshButton.tsx.

const RefreshButton: React.FC<{ onClick?: () => void }> = ({ onClick }) => {

frontend/src/ui-lib/Icons/RefreshIcon.tsx:34

The export name RefreshButton doesn't match the filename RefreshIcon.tsx. This should export RefreshIcon to maintain consistency.

export default RefreshButton;

Copilot · 2025-07-23T09:09:37Z

+            const isGraphMenu = item.keyId === "graph-library";
+
+            const handleClick = () => {
+              if (isGraphMenu) { openTab(graphTab === "run" ? "graph-library" : "graph-status"); }


[nitpick] The conditional logic should be moved to a separate line for better readability. Also consider extracting the ternary operation to make the intent clearer.

Suggested change

if (isGraphMenu) { openTab(graphTab === "run" ? "graph-library" : "graph-status"); }

if (isGraphMenu) {

const targetTab = graphTab === "run" ? "graph-library" : "graph-status";

openTab(targetTab);

}

Copilot · 2025-07-23T09:09:37Z

    return (
      <div className={styles.refreshButtonWrapper} data-testid="refresh-button">
-        <BlueButton onClick={() => fetchAllNodes()}>Refresh</BlueButton>
+        <button onClick={() => fetchAllNodes()} title="Refresh node parameters"> <RefreshIcon /> </button>


[nitpick] The button should use consistent styling classes instead of inline elements. Consider applying the same styling pattern used in other refresh buttons.

Copilot · 2025-07-23T09:09:38Z

    return (
      <div className={styles.buttonWrapper}>
-        <BlueButton onClick={() => fetchAllCalibrationGraphs(true)}>Refresh</BlueButton>
+        <button onClick={() => fetchAllCalibrationGraphs(true)} title="Refresh graph parameters"> <RefreshIcon /> </button>


[nitpick] The button should use consistent styling classes instead of inline elements. Consider applying the same styling pattern used in other refresh buttons.

Suggested change

<button onClick={() => fetchAllCalibrationGraphs(true)} title="Refresh graph parameters"> <RefreshIcon /> </button>

<button className={styles.refreshButton} onClick={() => fetchAllCalibrationGraphs(true)} title="Refresh graph parameters"> <RefreshIcon /> </button>

Adding the toggleSwitch to the topbarmenu

a8f0001

kleinwave self-assigned this Jun 30, 2025

kleinwave requested a review from manas-qm July 1, 2025 12:36

manas-qm reviewed Jul 2, 2025

View reviewed changes

addressing (some) review comments

cbf3f75

kleinwave requested a review from Copilot July 23, 2025 09:08

Copilot AI reviewed Jul 23, 2025

View reviewed changes

kleinwave added 3 commits July 23, 2025 15:53

Merge branch 'main' into feat/ToggleSwitch-to-TopBarMenu

3f02151

fixing linter warning

45c03d9

clicking graph menu card now also toggles the toggle switch

4c334fb

	<button onClick={() => fetchAllCalibrationGraphs(true)} title="Refresh graph parameters"> <RefreshIcon /> </button>
	<button className={styles.refreshButton} onClick={() => fetchAllCalibrationGraphs(true)} title="Refresh graph parameters"> <RefreshIcon /> </button>

Conversation

kleinwave commented Jun 30, 2025

Uh oh!

kleinwave commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

manas-qm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kleinwave commented Jun 30, 2025 •

edited

Loading