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

Sorting options - sort by size / duration / etc #108

Merged
merged 6 commits into from Jan 19, 2019
Merged

Conversation

whyboris
Copy link
Owner

Sort by:

  • default
  • size descending
  • size ascending
  • time descending
  • time ascending
  • random

⚠️ BREAKING CHANGES -- made some changes to the generated vha file.

TODO: In a future PR, simplify the silly shuffleTheViewNow hack and confirm that the old shuffle button and keyboard shortcuts work.

@whyboris whyboris requested a review from cal2195 January 19, 2019 04:15
@whyboris
Copy link
Owner Author

Please Squash and merge unless you have any reservations 👍

I'm now opting to pre- (and re-) computing the resolution string (and resolution sorting bucket) every time we open a vha file. This simplifies the resolution pipe somewhat, but also provides an important new feature: index. We no longer have to worry about using the hash as the unique reference to a file (in case a user has a duplicate file with the same size and therefore hash).

In the future we could refactor the index to be the unique reference when using the openVideo method.

@whyboris whyboris added the ✏️ ready for review PR is ready for review label Jan 19, 2019
This was referenced Jan 19, 2019
Copy link
Collaborator

@cal2195 cal2195 left a comment

Choose a reason for hiding this comment

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

I'm loving the idea of temporary variables generated at launch, but I don't think we should save them to the vha file - have I missed something, or what is your thoughts? 😄

main-support.ts Show resolved Hide resolved
src/app/components/common/final-object.interface.ts Outdated Show resolved Hide resolved
@whyboris
Copy link
Owner Author

I realized after submitting the PR and before falling asleep I should just remove the index, resolution string, and resolution bucket before saving the file.

Fixed with 87b01d6 🎉

@whyboris whyboris dismissed cal2195’s stale review January 19, 2019 12:42

Thank you :) - resolved with latest commit.

@whyboris whyboris added this to In progress in Boris's Current Workflow Jan 19, 2019
Copy link
Collaborator

@cal2195 cal2195 left a comment

Choose a reason for hiding this comment

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

Great work! 🎉

@whyboris
Copy link
Owner Author

I'll resolve the conflicts now and merge it 👍

@whyboris whyboris merged commit 5adc8c6 into master Jan 19, 2019
@whyboris whyboris moved this from In progress to Done in Boris's Current Workflow Jan 19, 2019
@whyboris whyboris deleted the sorting-options branch January 19, 2019 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✏️ ready for review PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants