-
Notifications
You must be signed in to change notification settings - Fork 328
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Creating directory in File Browser #12275
Conversation
🧪 Storybook is successfully deployed!📊 Dashboard:
|
The animation looks odd when it removes the placeholder and inserts the "real" entry. If we could use the same |
const updateDir = mutation('updateDirectory') | ||
|
||
function addNewDirectory() { | ||
assert(editedAsset.value == 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.
I don't see why we can be sure this will hold
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.
Good catch - the button should be disabled if editedAsset.value != null
console.error('Cannot rename directory without parentId') | ||
return | ||
} | ||
const requetsBody = { title: edited.name, parentId } |
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.
Typo
errorToast.show(`Failed to ${actionDescription}: ${error}`) | ||
}) | ||
.finally(() => { | ||
assert(edited === editedAsset.value) |
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.
Could this fail if the user pushes the create directory button before a previous directory creation is complete?
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.
As above - the button should be disabled before completion.
</template> | ||
</TransitionGroup> | ||
</div> | ||
<SvgButton name="folder_add" title="Add New Folder" @click.stop="addNewDirectory" /> |
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.
The title here and the placeholder name should be consistent: "New Folder" or "New Directory"?
Applied review. Now animation look much better - the screencast is updated. |
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.
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.
CR ✅ for shared/dashboard code
Pull Request Description
Part of #12113
Fixes #12237
Screencast.From.2025-02-14.12-32-10.mp4
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.