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

rename folder #563

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

rename folder #563

wants to merge 6 commits into from

Conversation

shubham13695
Copy link

@shubham13695 shubham13695 commented Oct 21, 2020

Closes #511

@whyboris whyboris self-requested a review October 21, 2020 17:08
@whyboris
Copy link
Owner

Really cool 🎉 Thank you for the PR 🤝 I'll try it out later today 🏃‍♂️

@whyboris whyboris mentioned this pull request Oct 21, 2020
Copy link
Owner

@whyboris whyboris left a comment

Choose a reason for hiding this comment

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

After a successful folder rename, do the videos in that folder remain playable?

I feel like unless we update all the paths for the videos in that folder, the videos in that folder will never be playable again (via clicking in the app) 🤔

@@ -1890,7 +1914,9 @@ export class HomeComponent implements OnInit, AfterViewInit {
this.currentRightClickedItem = item;
this.rightClickShowing = true;
}

ok(){
Copy link
Owner

Choose a reason for hiding this comment

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

WIP spandrel? 🙂

@whyboris
Copy link
Owner

@shubham13695 -- please let me know if you'd like to address the concern I raised or you'd rather I finish up the feature after merging 👌

At the moment I'm really tempted to get version 3.0.0 released - and this feature could be added after (so there's no rush now).

Thank you again for your contribution 🙇

@shubham13695
Copy link
Author

@whyboris yes I address it that after changing the folder on physical directories we need to update that change in our app. I m figuring out how to do it as rightClickElement index return -1 . if you have any idea please share it with me.

@whyboris
Copy link
Owner

I think this might only need a method in the ImageElementService to update all the objects in the imageElements array 🤔 (replacing the partialPath with the updated version of it).

So once the rename-folder-response comes back to Angular with a success, you would call that method 🤔

@shubham13695
Copy link
Author

hi @whyboris
I don't know what wrong with imageElementService.imageElements like before I update the file partial path it already has updated the folder with an older one same thing happening with the video file also.
if i update the same video file twice it create one more file.

@whyboris
Copy link
Owner

whyboris commented Oct 31, 2020

Cool! I checked out your branch -- somehow it creates the new elements (I'll look through your code again -- I didn't realize it did that somehow).

All you'll need to do is mark every element in the old folder path as deleted: true:
https://github.com/whyboris/Video-Hub-App/blob/main/interfaces/final-object.interface.ts#L52

This will prevent the element from being shown - and the element will be removed upon saving the file 🚀

ps - you might need to toggle the deletePipeHack (as I do all over the home.component.ts - to make sure all the deleted no longer show up in the gallery)

this.deletePipeHack = !this.deletePipeHack;

@whyboris
Copy link
Owner

btw -- if you happen to know -- could you point out how the new elements get created with your code? I'm looking all over your new code and I don't see anything that adds the videos from the new renamed folder into the app gallery - yet the videos do show up 🤯 :trollface:

I don't understand how that happens from your code -- I'm glad that it does, but could you let me know if you happen to know? 😊 😅

@whyboris
Copy link
Owner

I'm hoping to release version 3.0.0 this week. Because of limited times, and prioritizing the release, I chose not to include this feature in the initial 3.0.0 version.

I look forward to merging it in after a successful release of 3.0.0 🤝

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.

Rename folder
3 participants