Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: simplify workspace routing#17981

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
BrunoQuaresma merged 5 commits intomainfrombq/refactor-full-screen-layout
May 27, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletionssite/src/modules/dashboard/DashboardLayout.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,7 +8,6 @@ import { AnnouncementBanners } from "modules/dashboard/AnnouncementBanners/Annou
import { LicenseBanner } from "modules/dashboard/LicenseBanner/LicenseBanner";
import { type FC, type HTMLAttributes, Suspense } from "react";
import { Outlet } from "react-router-dom";
import { dashboardContentBottomPadding } from "theme/constants";
import { docs } from "utils/docs";
import { DeploymentBanner } from "./DeploymentBanner/DeploymentBanner";
import { Navbar } from "./Navbar/Navbar";
Expand All@@ -24,23 +23,10 @@ export const DashboardLayout: FC = () => {
{canViewDeployment && <LicenseBanner />}
<AnnouncementBanners />

<div
css={{
display: "flex",
minHeight: "100%",
flexDirection: "column",
}}
>
<div className="flex flex-col min-h-full">
<Navbar />

<div
css={{
flex: 1,
paddingBottom: dashboardContentBottomPadding, // Add bottom space since we don't use a footer
display: "flex",
flexDirection: "column",
}}
>
<div className="flex flex-col flex-1">
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried that we missed something in the conversion here:

  • The old version didn't treat this div as a flex item itself. It just defined how the item grows in the parent flex container
  • We didn't add any padding, when the old version of the element had it

Was that intentional?

Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

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

Yeap. I did some storybook reviews and QA around and I didn't find anything broken. Maybe it was a leftover? I would appreciate you helping me with some QA 🙏

<Suspense fallback={<Loader />}>
<Outlet />
</Suspense>
Expand DownExpand Up@@ -111,7 +97,6 @@ export const DashboardFullPage: FC<HTMLAttributes<HTMLDivElement>> = ({
<div
{...attrs}
css={{
marginBottom: `-${dashboardContentBottomPadding}px`,
flex: 1,
display: "flex",
flexDirection: "column",
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,17 +3,36 @@ import { deploymentStats } from "api/queries/deployment";
import { useAuthenticated } from "hooks";
import type { FC } from "react";
import { useQuery } from "react-query";
import { useLocation } from "react-router-dom";
import { DeploymentBannerView } from "./DeploymentBannerView";

const HIDE_DEPLOYMENT_BANNER_PATHS = [
// Hide the banner on workspace page because it already has a lot of
// information.
// - It adds names to the main groups that we're checking for, so it'll be a
// little more self-documenting
// - It redefines each group to only allow the characters A-Z (lowercase or
// uppercase), numbers, and hyphens
/^\/@(?<username>[a-zA-Z0-9-]+)\/(?<workspace_name>[a-zA-Z0-9-]+)$/,
];

export const DeploymentBanner: FC = () => {
const { permissions } = useAuthenticated();
const deploymentStatsQuery = useQuery(deploymentStats());
const healthQuery = useQuery({
...health(),
enabled: permissions.viewDeploymentConfig,
});
const location = useLocation();
const isHidden = HIDE_DEPLOYMENT_BANNER_PATHS.some((regex) =>
regex.test(location.pathname),
);

if (!permissions.viewDeploymentConfig || !deploymentStatsQuery.data) {
if (
isHidden ||
!permissions.viewDeploymentConfig ||
!deploymentStatsQuery.data
) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletionsite/src/modules/resources/useAgentLogs.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import { useAgentLogs } from "./useAgentLogs";
* Issue: https://github.com/romgain/jest-websocket-mock/issues/172
*/

describe("useAgentLogs", () => {
describe.skip("useAgentLogs", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Just got the go-ahead to work on fixing this flake today!

afterEach(() => {
WS.clean();
});
Expand Down
37 changes: 12 additions & 25 deletionssite/src/pages/WorkspacePage/WorkspacePage.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,8 +11,6 @@ import { displayError } from "components/GlobalSnackbar/utils";
import { Loader } from "components/Loader/Loader";
import { Margins } from "components/Margins/Margins";
import { useEffectEvent } from "hooks/hookPolyfills";
import { AnnouncementBanners } from "modules/dashboard/AnnouncementBanners/AnnouncementBanners";
import { Navbar } from "modules/dashboard/Navbar/Navbar";
import { type FC, useEffect } from "react";
import { useQuery, useQueryClient } from "react-query";
import { useParams } from "react-router-dom";
Expand DownExpand Up@@ -106,29 +104,18 @@ const WorkspacePage: FC = () => {
workspaceQuery.error ?? templateQuery.error ?? permissionsQuery.error;
const isLoading = !workspace || !template || !permissions;

return (
<>
<AnnouncementBanners />
<div css={{ height: "100%", display: "flex", flexDirection: "column" }}>
<Navbar />
{pageError ? (
<Margins>
<ErrorAlert
error={pageError}
css={{ marginTop: 16, marginBottom: 16 }}
/>
</Margins>
) : isLoading ? (
<Loader />
) : (
<WorkspaceReadyPage
workspace={workspace}
template={template}
permissions={permissions}
/>
)}
</div>
</>
return pageError ? (
<Margins>
<ErrorAlert error={pageError} css={{ marginTop: 16, marginBottom: 16 }} />
</Margins>
) : isLoading ? (
<Loader />
) : (
<WorkspaceReadyPage
workspace={workspace}
template={template}
permissions={permissions}
/>
);
};

Expand Down
25 changes: 12 additions & 13 deletionssite/src/router.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -532,20 +532,20 @@ export const router = createBrowserRouter(

{/* In order for the 404 page to work properly the routes that start with
top level parameter must be fully qualified. */}
<Route
path="/:username/:workspace/builds/:buildNumber"
element={<WorkspaceBuildPage />}
/>
<Route
path="/:username/:workspace/settings"
element={<WorkspaceSettingsLayout />}
>
<Route index element={<WorkspaceSettingsPage />} />
<Route path="/:username/:workspace">
<Route index element={<WorkspacePage />} />
<Route
path="parameters"
element={<WorkspaceParametersExperimentRouter />}
path="builds/:buildNumber"
element={<WorkspaceBuildPage />}
/>
<Route path="schedule" element={<WorkspaceSchedulePage />} />
<Route path="settings" element={<WorkspaceSettingsLayout />}>
<Route index element={<WorkspaceSettingsPage />} />
<Route
path="parameters"
element={<WorkspaceParametersExperimentRouter />}
/>
<Route path="schedule" element={<WorkspaceSchedulePage />} />
</Route>
</Route>

<Route path="/health" element={<HealthLayout />}>
Expand DownExpand Up@@ -574,7 +574,6 @@ export const router = createBrowserRouter(
</Route>

{/* Pages that don't have the dashboard layout */}
<Route path="/:username/:workspace" element={<WorkspacePage />} />
<Route
path="/templates/:template/versions/:version/edit"
element={<TemplateVersionEditorPage />}
Expand Down
1 change: 0 additions & 1 deletionsite/src/theme/constants.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,7 +32,6 @@ export const navHeight = 62;
export const containerWidth = 1380;
export const containerWidthMedium = 1080;
export const sidePadding = 24;
export const dashboardContentBottomPadding = 8 * 6;

// MUI does not have aligned heights for buttons and inputs so we have to "hack" it a little bit
export const BUTTON_XL_HEIGHT = 44;
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp