Skip to content

feat: Add a way to rename files from View header#588

Merged
going-confetti merged 5 commits intomainfrom
filename-header
Mar 31, 2025
Merged

feat: Add a way to rename files from View header#588
going-confetti merged 5 commits intomainfrom
filename-header

Conversation

@going-confetti
Copy link
Copy Markdown
Collaborator

@going-confetti going-confetti commented Mar 21, 2025

Closes #318

Description

Makes it much easier to rename a file, which should ideally nudge the users to give files names that are more meaningful than the auto generated ones.

Testing instructions

  • Try renaming all kinds of files (recordings, generators, scripts, data files)

interface FileNameHeaderProps {
file: StudioFile
isDirty?: boolean
showExt?: boolean
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

image


interface FileNameHeaderProps {
file: StudioFile
isDirty?: boolean
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

image

showExt?: boolean
}

export function FileNameHeader({
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if it's the best name, but we only have views that represent files, so I think it's fine

Comment on lines +35 to +36
fileName,
displayName: getFileNameWithoutExtension(fileName),
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Note: this is getting repetitive and is fairly error-prone - displayName is always (at least for now) the same as the file name without the extension, so it can be easily derived from it without the need to keep both values in sync.

<>
{getFileNameWithoutExtension(fileName)} {isDirty && '*'}
</>
<FileNameHeader
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This also fixes a recent regression that made it impossible to see the full file name if it overflows


const handleSave = async (newValue: string) => {
const newFileName = `${newValue}.${fileExtension}`
const newFileName = `${newValue.trim()}.${fileExtension}`
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Without this, it's easy to have multiple files with different names that look the same (e.g. generator.k6g and generator .k6g)

@going-confetti going-confetti changed the title wip: filename header feat: Add a way to rename files from View header Mar 26, 2025
@going-confetti going-confetti marked this pull request as ready for review March 26, 2025 15:42
@going-confetti going-confetti requested a review from a team as a code owner March 26, 2025 15:42
Comment thread src/components/FileNameHeader.tsx Outdated
onOpenChange: (open: boolean) => void
}

function RenameFileDialog({ file, open, onOpenChange }: RenameFileDialogProps) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to have some kind of an inline text input, but didn't have time to implement it the right way 🫤

Llandy3d
Llandy3d previously approved these changes Mar 26, 2025
Copy link
Copy Markdown
Collaborator

@Llandy3d Llandy3d left a comment

Choose a reason for hiding this comment

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

LGTM! 🙌

Copy link
Copy Markdown
Collaborator

@e-fisher e-fisher left a comment

Choose a reason for hiding this comment

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

Works great! 🙌 Have a few suggestions regarding UI:

  1. The dialog seems a bit overkill here, maybe Popover would be a better fit?

screencapture 2025-03-27 at 14 19 49

  1. Validation errors are displayed as toasts and a covered with dialog's overlay, inline validation could look better. Although, it becomes less of a problem if we switch to popover.

screencapture 2025-03-27 at 14 08 35

@going-confetti
Copy link
Copy Markdown
Collaborator Author

Works great! 🙌 Have a few suggestions regarding UI:

1. The dialog seems a bit overkill here, maybe Popover would be a better fit?

Looks better indeed 👍

2. Validation errors are displayed as toasts and a covered with dialog's overlay, inline validation could look better. Although, it becomes less of a problem if we switch to popover.

I'll switch to popover

}

return (
<Popover.Root open={isOpen} onOpenChange={setIsOpen}>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

image

@going-confetti going-confetti merged commit a4df3ad into main Mar 31, 2025
3 checks passed
@going-confetti going-confetti deleted the filename-header branch March 31, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make it possible to rename a file from the view title

3 participants