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

files views: Fix updateSelection() #3481

Merged
merged 2 commits into from
May 11, 2020
Merged

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented May 11, 2020

Boneheaded bug in my #3312 code.

@ferdnyc ferdnyc mentioned this pull request May 11, 2020
@ferdnyc ferdnyc merged commit cf69433 into OpenShot:develop May 11, 2020
@ferdnyc ferdnyc deleted the file-selection branch May 11, 2020 17:05
@jonoomph
Copy link
Member

@ferdnyc Some of this will likely be reverted with my changes to models and view (in the Emoji branch). It's difficult to reconcile the changes, so I just wanted to give you a heads up, that we might need to "re-implement" a few of these changes you've made to the file model and views after I'm done merging things. Or better yet, feel free to merge the relevant changes into the emoji branch, instead of develop. Sorry for the confusion!

@ferdnyc
Copy link
Contributor Author

ferdnyc commented May 15, 2020

@jonoomph Totally understand, and with improved files model code hopefully more of the view stuff becomes unnecessary anyway. (I've always thought the amount of duplicated code between the two views feels like a sign of things that really belong either in the model code, or somewhere else entirely. Selection-handling logic, in particular, can hopefully live with a selection model that's shared between the two views.)

I'll take a quick look now at the current emoji HEAD and see what's different from develop, figure out how/if the changes here and in the other PRs fit in.

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.

2 participants