Skip to content

fix(site): fix build logs scrolling on safari #14884

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

Merged
merged 7 commits into from
Oct 3, 2024
Merged

fix(site): fix build logs scrolling on safari #14884

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1adeb36
fix(site): fix build logs scrolling on safari
BrunoQuaresma Sep 30, 2024
c81a42b
Extract scroll area to encapsulate effect and state changes
BrunoQuaresma Oct 1, 2024
170bc1c
Use state to set height
BrunoQuaresma Oct 2, 2024
bbb93bb
Use height instead of width
BrunoQuaresma Oct 3, 2024
7a5f53e
Try to fix test
BrunoQuaresma Oct 3, 2024
8912122
Merge branch 'main' of https://github.com/coder/coder into bq/fix-bui…
BrunoQuaresma Oct 3, 2024
01e66ed
Fix tests
BrunoQuaresma Oct 3, 2024
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
Prev Previous commit
Next Next commit
Use state to set height
  • Loading branch information
@BrunoQuaresma
BrunoQuaresma committed Oct 2, 2024
commit 170bc1c2a4e780313c2a3ac2af1031b39ddb31d0
17 changes: 11 additions & 6 deletions site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@ import {
WorkspaceBuildDataSkeleton,
} from "modules/workspaces/WorkspaceBuildData/WorkspaceBuildData";
import { WorkspaceBuildLogs } from "modules/workspaces/WorkspaceBuildLogs/WorkspaceBuildLogs";
import { HTMLProps, useLayoutEffect, useRef, type FC } from "react";
import {
type CSSProperties,
type FC,
type HTMLProps,
useLayoutEffect,
useRef,
useState,
} from "react";
import { Link } from "react-router-dom";
import { displayWorkspaceBuildDuration } from "utils/workspace";
import { Sidebar, SidebarCaption, SidebarItem } from "./Sidebar";
Expand Down Expand Up @@ -212,6 +219,7 @@ const ScrollArea: FC<HTMLProps<HTMLDivElement>> = () => {
// Issue: https://github.com/coder/coder/issues/9687
// Reference: https://stackoverflow.com/questions/43381836/height100-works-in-chrome-but-not-in-safari
const contentRef = useRef<HTMLDivElement>(null);
const [height, setHeight] = useState<CSSProperties["width"]>("100%");
Copy link
Member

Choose a reason for hiding this comment

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

Tiny thing, but could we set the type to be based on CSSProperties["height"]>?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lol yes, idk why I used width 🤦

useLayoutEffect(() => {
const contentEl = contentRef.current;
if (!contentEl) {
Expand All @@ -223,7 +231,7 @@ const ScrollArea: FC<HTMLProps<HTMLDivElement>> = () => {
if (!parentEl) {
return;
}
contentEl.style.setProperty("height", `${parentEl.clientHeight}px`);
setHeight(parentEl.clientHeight);
});
resizeObserver.observe(document.body);

Expand All @@ -233,10 +241,7 @@ const ScrollArea: FC<HTMLProps<HTMLDivElement>> = () => {
}, []);

return (
<div
ref={contentRef}
css={{ height: "100%", overflowY: "auto", width: "100%" }}
/>
<div ref={contentRef} css={{ height, overflowY: "auto", width: "100%" }} />
);
};

Expand Down
Loading