Skip to content

new-log-viewer: Resize panel width when the browser window resizes. #94

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 12 commits into from
Oct 22, 2024
Merged

new-log-viewer: Resize panel width when the browser window resizes. #94

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b5e278e
Enforce a min width on the editor in place of the previous max panel …
junhaoliao Oct 15, 2024
337e013
Refactor panel width setter / getters to use documentElement; getComp…
junhaoliao Oct 15, 2024
740080e
Change PANEL_CLIP_THRESHOLD_IN_PIXELS to 360 and EDITOR_MIN_WIDTH_IN_…
junhaoliao Oct 21, 2024
9101e31
Merge branch 'main' into panel-window-resize
junhaoliao Oct 21, 2024
b6926de
Change panel sizes.
junhaoliao Oct 21, 2024
7e52a9d
Revert changes on sidebar / panel sizes.
junhaoliao Oct 21, 2024
8eb1b90
Include `tabListRef.current.clientWidth` into `PANEL_CLIP_THRESHOLD_I…
junhaoliao Oct 21, 2024
3caf68e
Refactor sidebar resizing logic.
junhaoliao Oct 21, 2024
0db470e
Adjust sidebar panel width before setting it; change PANEL_CLIP_THRES…
junhaoliao Oct 21, 2024
9dcabbc
Revise the logic for setting panel width to ensure it does not exceed…
junhaoliao Oct 22, 2024
7d23a26
Adjust panel open width logic.
junhaoliao Oct 22, 2024
a1c812d
Clamp the panel width also during resizes.
junhaoliao Oct 22, 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
73 changes: 51 additions & 22 deletions new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
getConfig,
setConfig,
} from "../../../utils/config";
import {clamp} from "../../../utils/math";
import ResizeHandle from "./ResizeHandle";
import SidebarTabs from "./SidebarTabs";

Expand All @@ -20,15 +21,15 @@ import "./index.css";

const PANEL_DEFAULT_WIDTH_IN_PIXELS = 360;
const PANEL_CLIP_THRESHOLD_IN_PIXELS = 250;
const PANEL_MAX_WIDTH_TO_WINDOW_WIDTH_RATIO = 0.8;
const EDITOR_MIN_WIDTH_IN_PIXELS = 250;

/**
* Gets width of the panel from body style properties.
*
* @return the width in pixels as a number.
*/
const getPanelWidth = () => parseInt(
document.body.style.getPropertyValue("--ylv-panel-width"),
getComputedStyle(document.documentElement).getPropertyValue("--ylv-panel-width"),
10
);

Expand All @@ -38,10 +39,9 @@ const getPanelWidth = () => parseInt(
* @param newValue in pixels.
*/
const setPanelWidth = (newValue: number) => {
document.body.style.setProperty("--ylv-panel-width", `${newValue}px`);
document.documentElement.style.setProperty("--ylv-panel-width", `${newValue}px`);
};


/**
* Renders a sidebar component that displays tabbed panels and a resize handle.
* The active tab can be changed and the sidebar can be resized by dragging the handle.
Expand All @@ -60,15 +60,24 @@ const Sidebar = () => {
return;
}

let newTabName = tabName;
let newPanelWidth = PANEL_DEFAULT_WIDTH_IN_PIXELS;
if (activeTabName === tabName) {
newTabName = TAB_NAME.NONE;
newPanelWidth = tabListRef.current.clientWidth;
// Close the panel
setActiveTabName(TAB_NAME.NONE);
setConfig({key: CONFIG_KEY.INITIAL_TAB_NAME, value: TAB_NAME.NONE});
setPanelWidth(tabListRef.current.clientWidth);

return;
}
setActiveTabName(newTabName);
setConfig({key: CONFIG_KEY.INITIAL_TAB_NAME, value: newTabName});
setPanelWidth(newPanelWidth);

setActiveTabName(tabName);
setConfig({key: CONFIG_KEY.INITIAL_TAB_NAME, value: tabName});
setPanelWidth(
clamp(
window.innerWidth - EDITOR_MIN_WIDTH_IN_PIXELS,
PANEL_CLIP_THRESHOLD_IN_PIXELS,
PANEL_DEFAULT_WIDTH_IN_PIXELS
)
);
}, [activeTabName]);

const handleResizeHandleRelease = useCallback(() => {
Expand All @@ -83,23 +92,43 @@ const Sidebar = () => {

return;
}
if (
tabListRef.current.clientWidth + PANEL_CLIP_THRESHOLD_IN_PIXELS >
resizeHandlePosition
) {
if (PANEL_CLIP_THRESHOLD_IN_PIXELS > resizeHandlePosition) {
Copy link
Contributor

@davemarco davemarco Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

See suggestion below since can't suggest

    const handleResize = useCallback((resizeHandlePosition: number) => {
        if (null === tabListRef.current) {
            console.error("Unexpected null tabListRef.current");

            return;
        }
        const maxHandleXPos = Math.max(window.innerWidth - EDITOR_MIN_WIDTH_IN_PIXELS, PANEL_DEFAULT_WIDTH_IN_PIXELS);
        if (resizeHandlePosition < PANEL_CLIP_THRESHOLD_IN_PIXELS) {
            // If the resize handle is positioned to the right of the <TabList/>'s right edge
            // with a clipping threshold accounted, close the panel.
            setPanelWidth(tabListRef.current.clientWidth);
        } else if (resizeHandlePosition < maxHandleXPos) {
            // Update the panel width with the distance between the mouse pointer and the window's
            // left edge.
            setPanelWidth(resizeHandlePosition);
        }
    }, []);

Copy link
Member Author

Choose a reason for hiding this comment

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

Similar to the one in handleActiveTabNameChange, I feel we want to use Math.min(newPanelWidth, window.innerWidth - EDITOR_MIN_WIDTH_IN_PIXELS)

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like that line in handleActiveTabNameChange is dangerous, as can set below the clipping width. The max here is meant to solve issues where window.innerWidth - EDITOR_MIN_WIDTH_IN_PIXELS calculation is flawed. When the window.innerwidth is small, the actual editor width is smaller than EDITOR_MIN_WIDTH_IN_PIXELS. This can give very small values nonsensical values for maxHandleXPos/available width. Say the innerwidth is 400, the editor min width is 360. Then the maxHandleXPos/avaialble is 40. However, in reality. the editor is probably only taking up 40px, and the available width should be 360.

However, im sure my calc is flawed in other ways... but for now it seems to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think we should come up with a better fix for this later, and just approve something that works for now

Copy link
Member Author

@junhaoliao junhaoliao Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

that line in handleActiveTabNameChange is dangerous, as can set below the clipping width

I agree. Let's fix it at least in a way that mobile users can read the content in the panel; we can optimize the experience in a later PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I havn't tested on mobile... if my comment was unclear i basically just want to keep the max

Copy link
Member Author

Choose a reason for hiding this comment

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

right, we kept the max and also added a min:

        clamp(
            window.innerWidth - EDITOR_MIN_WIDTH_IN_PIXELS,
            PANEL_CLIP_THRESHOLD_IN_PIXELS,
            PANEL_DEFAULT_WIDTH_IN_PIXELS
        )

FYR

/**
 * Clamps a number to the given range. E.g. If the number is greater than the range's upper bound,
 * the range's upper bound is returned.
 *
 * @param num The number to be clamped.
 * @param min The lower boundary of the output range.
 * @param max The upper boundary of the output range.
 * @return The clamped number.
 */
const clamp = (num: number, min: number, max: number) => Math.min(Math.max(num, min), max);

// If the resize handle is positioned to the right of the <TabList/>'s right edge
// with a clipping threshold accounted, close the panel.
setPanelWidth(tabListRef.current.clientWidth);
} else if (
resizeHandlePosition < window.innerWidth * PANEL_MAX_WIDTH_TO_WINDOW_WIDTH_RATIO
) {
// If the resize handle is positioned to the left of 80% of the window's width,
// update the panel width with the distance between the mouse pointer and the
// window's left edge.
setPanelWidth(resizeHandlePosition);
} else {
// Update the panel width with the distance between the mouse pointer and the window's
// left edge.
setPanelWidth(
clamp(
window.innerWidth - EDITOR_MIN_WIDTH_IN_PIXELS,
PANEL_CLIP_THRESHOLD_IN_PIXELS,
resizeHandlePosition
)
);
}
}, []);

// On initialization, register window resize event handler to resize panel width when necessary.
useEffect(() => {
const handleWindowResize = () => {
const availableWidth = Math.max(
window.innerWidth - EDITOR_MIN_WIDTH_IN_PIXELS,
PANEL_CLIP_THRESHOLD_IN_PIXELS
);

if (getPanelWidth() > availableWidth) {
setPanelWidth(availableWidth);
}
};

window.addEventListener("resize", handleWindowResize);

return () => {
window.removeEventListener("resize", handleWindowResize);
};
}, []);

// On initialization, do not show panel if there is no active tab.
useEffect(() => {
if (null === tabListRef.current) {
Expand Down
Loading