-
Notifications
You must be signed in to change notification settings - Fork 642
UX Improvements: Folder-based tab titles, scroll enhancements, and file preview features #2674
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f76712c
e08c0cb
85858e1
da2595c
7b73a2d
da20c87
da450d0
f6e27d2
0709c3d
ec04394
06b2038
cf1c511
048b7a1
d59fe56
e4164c4
6339407
cde3b7b
58fbaac
855788d
c3be1cf
7e25cb6
e4d2df6
7d9a0a5
56e7c17
069fa4c
a378f7b
fe684c9
6c3404f
def8d03
c570bf3
4970ac4
18ab50f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,39 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Copyright 2025, Command Line Inc. | ||||||||||||||||||||||||||||||||||||||||||||||||
| // SPDX-License-Identifier: Apache-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import { Modal } from "@/app/modals/modal"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { deleteLayoutModelForTab } from "@/layout/index"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { atoms, getApi, globalStore } from "@/store/global"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { modalsModel } from "@/store/modalmodel"; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| interface ConfirmCloseTabModalProps { | ||||||||||||||||||||||||||||||||||||||||||||||||
| tabId: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const ConfirmCloseTabModal = ({ tabId }: ConfirmCloseTabModalProps) => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const handleConfirmClose = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const ws = globalStore.get(atoms.workspace); | ||||||||||||||||||||||||||||||||||||||||||||||||
| getApi().closeTab(ws.oid, tabId); | ||||||||||||||||||||||||||||||||||||||||||||||||
| deleteLayoutModelForTab(tabId); | ||||||||||||||||||||||||||||||||||||||||||||||||
| modalsModel.popModal(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling and await async operations. The Additionally, there's no validation that the workspace exists or that the Consider this refactor: - const handleConfirmClose = () => {
+ const handleConfirmClose = async () => {
+ try {
const ws = globalStore.get(atoms.workspace);
- getApi().closeTab(ws.oid, tabId);
+ if (!ws?.oid || !tabId) {
+ console.error("Invalid workspace or tabId");
+ modalsModel.popModal();
+ return;
+ }
+ await getApi().closeTab(ws.oid, tabId);
deleteLayoutModelForTab(tabId);
modalsModel.popModal();
+ } catch (error) {
+ console.error("Failed to close tab:", error);
+ // Consider showing an error message to the user
+ modalsModel.popModal();
+ }
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const handleCancel = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| modalsModel.popModal(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||
| <Modal onOk={handleConfirmClose} onCancel={handleCancel} onClose={handleCancel}> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="content"> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="modal-title">Close Tab?</div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div style={{ marginTop: "10px" }}> | ||||||||||||||||||||||||||||||||||||||||||||||||
| Are you sure you want to close this tab? This action cannot be undone. | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </Modal> | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| ConfirmCloseTabModal.displayName = "ConfirmCloseTabModal"; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export { ConfirmCloseTabModal }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,13 +350,35 @@ export class PreviewModel implements ViewModel { | |
| title: "Table of Contents", | ||
| click: () => this.markdownShowTocToggle(), | ||
| }, | ||
| { | ||
| elemtype: "iconbutton", | ||
| icon: "arrows-rotate", | ||
| title: "Refresh", | ||
| click: () => this.refreshCallback?.(), | ||
| }, | ||
| ] as IconButtonDecl[]; | ||
| } else if (!isCeView && mimeType) { | ||
| // For all other file types (text, code, etc.), add refresh button | ||
| return [ | ||
| { | ||
| elemtype: "iconbutton", | ||
| icon: "arrows-rotate", | ||
| title: "Refresh", | ||
| click: () => this.refreshCallback?.(), | ||
| }, | ||
| ] as IconButtonDecl[]; | ||
|
Comment on lines
+353
to
369
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: rg -n "refreshCallback\s*=" frontend/app/view/preview/Repository: wavetermdev/waveterm Length of output: 789 🏁 Script executed: rg -n "refreshCallback" frontend/app/view/preview/ -A 3 -B 3Repository: wavetermdev/waveterm Length of output: 9670 🏁 Script executed: fd . frontend/app/view/preview -name "*.tsx" -o -name "*.ts" | head -20Repository: wavetermdev/waveterm Length of output: 294 Fix unsafe refreshCallback invocations in preview-directory operations. The refresh buttons correctly use optional chaining 🤖 Prompt for AI Agents |
||
| } | ||
| return null; | ||
| }); | ||
| this.metaFilePath = atom<string>((get) => { | ||
| const file = get(this.blockAtom)?.meta?.file; | ||
| const blockData = get(this.blockAtom); | ||
| const file = blockData?.meta?.file; | ||
| if (isBlank(file)) { | ||
| // If no file is set, default to the terminal's current working directory | ||
| const cwd = blockData?.meta?.[waveobj.MetaKey_CmdCwd]; | ||
| if (!isBlank(cwd)) { | ||
| return cwd; | ||
| } | ||
| return "~"; | ||
| } | ||
| return file; | ||
|
|
@@ -408,6 +430,7 @@ export class PreviewModel implements ViewModel { | |
| this.goParentDirectory = this.goParentDirectory.bind(this); | ||
|
|
||
| const fullFileAtom = atom<Promise<FileData>>(async (get) => { | ||
| get(this.refreshVersion); // Subscribe to refreshVersion to trigger re-fetch | ||
| const fileName = get(this.metaFilePath); | ||
| const path = await this.formatRemoteUri(fileName, get); | ||
| if (fileName == null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 837
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 3173
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 5151
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 91
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 1039
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 2001
Edge case:
userHasScrolledUppersists across conversation resets.When a user scrolls up in one conversation, then clears the chat or starts a new conversation (via
clearChat()which callsuseChatSetMessages([])and resets the messages array), theuserHasScrolledUpflag remainstrue. This prevents auto-scrolling to new messages in the fresh conversation since the effect at lines 67-72 only callsscrollToBottom()when!userHasScrolledUp.current. The flag only resets when the user manually scrolls near the bottom orscrollToBottom()is explicitly called. Consider resetting this flag when the conversation changes—for instance, by detecting whenmessages.length === 0within the messages effect, or by adding an explicit reset trigger.🤖 Prompt for AI Agents