Skip to content

Commit

Permalink
Squash some leftover bugs (#1495)
Browse files Browse the repository at this point in the history
Only create new tab in `CheckAndFixWindow` if no tabs or pinned tabs
exist

Update `resolvers.resolveTabNum` to account for pinned tabs

Remove obsolete and unused `wstore.DeleteTab`

Only show accelerators for first 9 workspaces in workspace app menu to
be consistent with other keybindings

Fix tabbar spacing to remove min size for drag right spacer, account for
workspace switcher button size

Fix updatebanner size calculations
  • Loading branch information
esimkowitz authored Dec 11, 2024
1 parent 87ebbdd commit 4070aba
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 70 deletions.
Binary file added build/icons/48x48.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added build/icons/64x64.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 2 additions & 7 deletions emain/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,8 @@ async function getWorkspaceMenu(ww?: WaveBrowserWindow): Promise<Electron.MenuIt
},
];
function getWorkspaceSwitchAccelerator(i: number): string {
if (i < 10) {
if (i == 9) {
i = 0;
} else {
i++;
}
return unamePlatform == "darwin" ? `Command+Control+${i}` : `Alt+Control+${i}`;
if (i < 9) {
return unamePlatform == "darwin" ? `Command+Control+${i}` : `Alt+Control+${i + 1}`;
}
}
workspaceList?.length &&
Expand Down
14 changes: 7 additions & 7 deletions frontend/app/tab/tabbar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,20 @@
user-select: none;
display: flex;
flex-direction: row;
width: env(titlebar-area-width);
-webkit-app-region: drag;
width: 100vw;
-webkit-app-region: drag;

* {
-webkit-app-region: no-drag;
}
* {
-webkit-app-region: no-drag;
}

.tabs-wrapper {
transition: var(--tabs-wrapper-transition);
height: 26px;
}

.tab-bar {
margin-top: 6px;
margin-top: 6px;
position: relative; // Needed for absolute positioning of child tabs
display: flex;
flex-direction: row;
Expand All @@ -61,7 +61,7 @@
display: flex;
align-items: center;
justify-content: center;
margin: 6px 6px 0 0;
padding: 6px 6px 0 0;
}

.app-menu-button {
Expand Down
29 changes: 22 additions & 7 deletions frontend/app/tab/tabbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { WorkspaceSwitcher } from "./workspaceswitcher";

const TAB_DEFAULT_WIDTH = 130;
const TAB_MIN_WIDTH = 100;
const DRAGGER_RIGHT_MIN_WIDTH = 74;
const OS_OPTIONS = {
overflow: {
x: "scroll",
Expand Down Expand Up @@ -168,7 +167,7 @@ const TabBar = memo(({ workspace }: TabBarProps) => {
const appMenuButtonRef = useRef<HTMLDivElement>(null);
const tabWidthRef = useRef<number>(TAB_DEFAULT_WIDTH);
const scrollableRef = useRef<boolean>(false);
const updateStatusButtonRef = useRef<HTMLButtonElement>(null);
const updateStatusBannerRef = useRef<HTMLDivElement>(null);
const configErrorButtonRef = useRef<HTMLElement>(null);
const prevAllLoadedRef = useRef<boolean>(false);
const activeTabId = useAtomValue(atoms.staticTabId);
Expand Down Expand Up @@ -227,16 +226,17 @@ const TabBar = memo(({ workspace }: TabBarProps) => {

const tabbarWrapperWidth = tabbarWrapperRef.current.getBoundingClientRect().width;
const windowDragLeftWidth = draggerLeftRef.current.getBoundingClientRect().width;
const windowDragRightWidth = draggerRightRef.current.getBoundingClientRect().width;
const addBtnWidth = addBtnRef.current.getBoundingClientRect().width;
const updateStatusLabelWidth = updateStatusButtonRef.current?.getBoundingClientRect().width ?? 0;
const updateStatusLabelWidth = updateStatusBannerRef.current?.getBoundingClientRect().width ?? 0;
const configErrorWidth = configErrorButtonRef.current?.getBoundingClientRect().width ?? 0;
const appMenuButtonWidth = appMenuButtonRef.current?.getBoundingClientRect().width ?? 0;
const workspaceSwitcherWidth = workspaceSwitcherRef.current?.getBoundingClientRect().width ?? 0;
const devLabelWidth = devLabelRef.current?.getBoundingClientRect().width ?? 0;

const nonTabElementsWidth =
windowDragLeftWidth +
DRAGGER_RIGHT_MIN_WIDTH +
windowDragRightWidth +
addBtnWidth +
updateStatusLabelWidth +
configErrorWidth +
Expand All @@ -256,6 +256,21 @@ const TabBar = memo(({ workspace }: TabBarProps) => {
// Determine if the tab bar needs to be scrollable
const newScrollable = idealTabWidth * numberOfTabs > spaceForTabs;

console.log(
"tabbarWrapperWidth",
tabbarWrapperWidth,
"nonTabElementsWidth",
nonTabElementsWidth,
"idealTabWidth",
idealTabWidth,
"spaceForTabs",
spaceForTabs,
"newScrollable",
newScrollable,
"devLabelWidth",
devLabelWidth
);

// Apply the calculated width and position to all tabs
tabRefs.current.forEach((ref, index) => {
if (ref.current) {
Expand Down Expand Up @@ -653,7 +668,7 @@ const TabBar = memo(({ workspace }: TabBarProps) => {
<WindowDrag ref={draggerLeftRef} className="left" />
{appMenuButton}
{devLabel}
<WorkspaceSwitcher />
<WorkspaceSwitcher ref={workspaceSwitcherRef} />
<div className="tab-bar" ref={tabBarRef} data-overlayscrollbars-initialize>
<div className="tabs-wrapper" ref={tabsWrapperRef} style={{ width: `${tabsWrapperWidth}px` }}>
{tabIds.map((tabId, index) => {
Expand Down Expand Up @@ -683,8 +698,8 @@ const TabBar = memo(({ workspace }: TabBarProps) => {
<div ref={addBtnRef} className="add-tab-btn" onClick={handleAddTab}>
<i className="fa fa-solid fa-plus fa-fw" />
</div>
<WindowDrag ref={draggerRightRef} className="right" style={{ minWidth: DRAGGER_RIGHT_MIN_WIDTH }} />
<UpdateStatusBanner buttonRef={updateStatusButtonRef} />
<WindowDrag ref={draggerRightRef} className="right" />
<UpdateStatusBanner ref={updateStatusBannerRef} />
<ConfigErrorIcon buttonRef={configErrorButtonRef} />
</div>
);
Expand Down
20 changes: 12 additions & 8 deletions frontend/app/tab/updatebanner.scss
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
.update-available-button {
height: 80%;
margin: auto 4px;
color: black;
background-color: var(--accent-color);
flex: 0 0 fit-content;
.update-available-banner {
display: flex;
height: 100%;
.button {
color: black;
height: 80%;
margin: auto 4px;
background-color: var(--accent-color);
flex: 0 0 fit-content;

&:disabled {
opacity: unset !important;
&:disabled {
opacity: unset !important;
}
}
}
25 changes: 13 additions & 12 deletions frontend/app/tab/updatebanner.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Button } from "@/element/button";
import { atoms, getApi } from "@/store/global";
import { useAtomValue } from "jotai";
import { memo, useEffect, useState } from "react";
import { forwardRef, memo, useEffect, useState } from "react";
import "./updatebanner.scss";

const UpdateStatusBannerComponent = ({ buttonRef }: { buttonRef: React.RefObject<HTMLButtonElement> }) => {
const UpdateStatusBannerComponent = forwardRef<HTMLDivElement>((_, ref) => {
const appUpdateStatus = useAtomValue(atoms.updaterStatusAtom);
let [updateStatusMessage, setUpdateStatusMessage] = useState<string>();
const [dismissBannerTimeout, setDismissBannerTimeout] = useState<NodeJS.Timeout>();
Expand Down Expand Up @@ -54,17 +54,18 @@ const UpdateStatusBannerComponent = ({ buttonRef }: { buttonRef: React.RefObject
}
if (updateStatusMessage) {
return (
<Button
ref={buttonRef}
className="update-available-button"
title={appUpdateStatus === "ready" ? "Click to Install Update" : updateStatusMessage}
onClick={onClick}
disabled={appUpdateStatus !== "ready"}
>
{updateStatusMessage}
</Button>
<div className="update-available-banner" ref={ref}>
<Button
className="update-available-button"
title={appUpdateStatus === "ready" ? "Click to Install Update" : updateStatusMessage}
onClick={onClick}
disabled={appUpdateStatus !== "ready"}
>
{updateStatusMessage}
</Button>
</div>
);
}
};
});

export const UpdateStatusBanner = memo(UpdateStatusBannerComponent) as typeof UpdateStatusBannerComponent;
7 changes: 4 additions & 3 deletions frontend/app/tab/workspaceswitcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import clsx from "clsx";
import { atom, PrimitiveAtom, useAtom, useAtomValue, useSetAtom } from "jotai";
import { splitAtom } from "jotai/utils";
import { OverlayScrollbarsComponent } from "overlayscrollbars-react";
import { CSSProperties, memo, useCallback, useEffect, useRef } from "react";
import { CSSProperties, forwardRef, memo, useCallback, useEffect, useRef } from "react";
import WorkspaceSVG from "../asset/workspace.svg";
import { IconButton } from "../element/iconbutton";
import { atoms, getApi } from "../store/global";
Expand Down Expand Up @@ -167,7 +167,7 @@ type WorkspaceList = WorkspaceListEntry[];
const workspaceMapAtom = atom<WorkspaceList>([]);
const workspaceSplitAtom = splitAtom(workspaceMapAtom);
const editingWorkspaceAtom = atom<string>();
const WorkspaceSwitcher = () => {
const WorkspaceSwitcher = forwardRef<HTMLDivElement>((_, ref) => {
const setWorkspaceList = useSetAtom(workspaceMapAtom);
const activeWorkspace = useAtomValueSafe(atoms.workspace);
const workspaceList = useAtomValue(workspaceSplitAtom);
Expand Down Expand Up @@ -231,6 +231,7 @@ const WorkspaceSwitcher = () => {
className="workspace-switcher-popover"
placement="bottom-start"
onDismiss={() => setEditingWorkspace(null)}
ref={ref}
>
<PopoverButton
className="workspace-switcher-button grey"
Expand Down Expand Up @@ -271,7 +272,7 @@ const WorkspaceSwitcher = () => {
</PopoverContent>
</Popover>
);
};
});

const WorkspaceSwitcherItem = ({
entryAtom,
Expand Down
2 changes: 1 addition & 1 deletion pkg/wcore/window.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func CheckAndFixWindow(ctx context.Context, windowId string) *waveobj.Window {
CloseWindow(ctx, windowId, false)
return nil
}
if len(ws.TabIds) == 0 {
if len(ws.TabIds) == 0 && len(ws.PinnedTabIds) == 0 {
log.Printf("fixing workspace with no tabs %q (in checkAndFixWindow)\n", ws.OID)
_, err = CreateTab(ctx, ws.OID, "", true, false, false)
if err != nil {
Expand Down
14 changes: 11 additions & 3 deletions pkg/wshrpc/wshserver/resolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,19 @@ func resolveTabNum(ctx context.Context, data wshrpc.CommandResolveIdsData, value
return nil, fmt.Errorf("error getting workspace: %v", err)
}

if tabNum < 1 || tabNum > len(ws.TabIds) {
return nil, fmt.Errorf("tab num out of range, workspace has %d tabs", len(ws.TabIds))
numPinnedTabs := len(ws.PinnedTabIds)
numTabs := len(ws.TabIds) + numPinnedTabs
if tabNum < 1 || tabNum > numTabs {
return nil, fmt.Errorf("tab num out of range, workspace has %d tabs", numTabs)
}

resolvedTabId := ws.TabIds[tabNum-1]
tabIdx := tabNum - 1
var resolvedTabId string
if tabIdx < numPinnedTabs {
resolvedTabId = ws.PinnedTabIds[tabIdx]
} else {
resolvedTabId = ws.TabIds[tabIdx-numPinnedTabs]
}
return &waveobj.ORef{OType: waveobj.OType_Tab, OID: resolvedTabId}, nil
}

Expand Down
22 changes: 0 additions & 22 deletions pkg/wstore/wstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,6 @@ func UpdateTabName(ctx context.Context, tabId, name string) error {
})
}

// must delete all blocks individually first
// also deletes LayoutState
func DeleteTab(ctx context.Context, workspaceId string, tabId string) error {
return WithTx(ctx, func(tx *TxWrap) error {
tab, _ := DBGet[*waveobj.Tab](tx.Context(), tabId)
if tab == nil {
return nil
}
if len(tab.BlockIds) != 0 {
return fmt.Errorf("tab has blocks, must delete blocks first")
}
ws, _ := DBGet[*waveobj.Workspace](tx.Context(), workspaceId)
if ws != nil {
ws.TabIds = utilfn.RemoveElemFromSlice(ws.TabIds, tabId)
DBUpdate(tx.Context(), ws)
}
DBDelete(tx.Context(), waveobj.OType_Tab, tabId)
DBDelete(tx.Context(), waveobj.OType_LayoutState, tab.LayoutState)
return nil
})
}

func UpdateObjectMeta(ctx context.Context, oref waveobj.ORef, meta waveobj.MetaMapType, mergeSpecial bool) error {
return WithTx(ctx, func(tx *TxWrap) error {
if oref.IsEmpty() {
Expand Down

0 comments on commit 4070aba

Please sign in to comment.