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

dev: adding sorting for TrackSelectionDialog and TrackSelectionDialog… #7798

Closed
wants to merge 7 commits into from

Conversation

yoobi
Copy link
Contributor

@yoobi yoobi commented Aug 24, 2020

This enhancement match with this issue #7709

This is my first time contributing to a project, feel free to tell me if I did something wrong.

Developers need to add their comparator which is type of Comparator<Format> and it'll sort it the way they implement it

PS: I got an issue with my repo and had to delete it, that's why there is 2 merge request, feel free to close/delete this one: #7715

@google-cla google-cla bot added the cla: yes label Aug 24, 2020
@ojw28 ojw28 self-assigned this Aug 24, 2020
Copy link
Contributor

@ojw28 ojw28 left a comment

Choose a reason for hiding this comment

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

I put some comments inline. For the demo app changes, I think we probably don't want to take those, since it makes the demo app quite a bit more complicated. Feel free to leave them in for now if they're useful for your own testing, but let's remove them when we merge.

@yoobi
Copy link
Contributor Author

yoobi commented Sep 9, 2020

Everything should be okay now, I'm only concern if for some reason a TrackGroup has a length of 0, then there will be some issue, I haven't been able to find a TrackGroupArray with an empty TrackGroup in the demo app though

ojw28 added a commit that referenced this pull request Sep 25, 2020
@ojw28 ojw28 closed this Sep 25, 2020
ojw28 added a commit that referenced this pull request Oct 21, 2020
@google google locked and limited conversation to collaborators Nov 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants