Skip to content
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

feat: rename file #102

Merged
merged 5 commits into from
Aug 27, 2021
Merged

feat: rename file #102

merged 5 commits into from
Aug 27, 2021

Conversation

lxkuz
Copy link
Contributor

@lxkuz lxkuz commented Jul 22, 2021

Describe what changes this pull request brings

Covering #92 we add a dropdown to the file with Delete and Rename options.

Describe any additional changes

Rename modal is generalized and now supports both file and folder.

Steps to test

  • create a file in Snipsnap admin UI
  • try to rename this file, check result persists after saving
  • try to delete this file
  • regression testing that folders management is not broken

Screenshots
Screenshot 2021-07-23 at 02 40 54

Screenshot 2021-07-23 at 02 49 02

Checklist

  • I've checked for typos
  • I've made sure overall structure consistency is preserved
  • I've made sure overall headings hierarchy is preserved
  • I've reread proposed changes twice

TODO

References
Issue #92

@lxkuz lxkuz requested review from lnikell and bolotskydev July 22, 2021 21:58
@lxkuz lxkuz changed the title Feat/rename file feat: rename file Jul 22, 2021
@bolotskydev bolotskydev self-assigned this Jul 23, 2021
@bolotskydev bolotskydev added the enhancement New feature or request label Jul 23, 2021
Comment on lines 98 to 102
let newData = cloneDeep(files);
newData = newData.map((item) =>
item.id === fileId ? { ...item, data: { ...item.data, name: newName } } : item
);
return { files: newData };
Copy link

@bolotskydev bolotskydev Jul 30, 2021

Choose a reason for hiding this comment

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

what is the reason of deep clone here? right below you are essentially recreating the whole thing anyway 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reworked this totally, review again, please.

@bolotskydev
Copy link

@lxkuz thanks for contribution! Made some nitpicks, otherwise LGTM! GJ 👏

@lxkuz lxkuz requested a review from bolotskydev August 1, 2021 20:34
@lxkuz lxkuz merged commit 7801841 into master Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants