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

Fix https://github.com/ubports/libertine/issues/33 #34

Merged
merged 3 commits into from May 28, 2019

Conversation

@arubislander
Copy link

commented Apr 7, 2019

After the transition from PageStack to AdaptivePageLayout for the Libertine configuration views some more fixes where needed:
Page.pageStack.currentPage is undefined for an AdaptivePageLayout so the installPackage() method on the ContainerEditView was no longer being called.

I added a property homeView to DebianPackagePicker and SearchResultsView, pass the instance of ContainerEditView (which has the id of homeView) to them and access the method via this property.

Terence Sambo

@dobey dobey added this to In progress in OTA-9 via automation Apr 12, 2019

@dobey

This comment has been minimized.

Copy link
Member

commented Apr 12, 2019

Hmm. I think it would be better instead to provide a common signal which the pages emit, and which is connected when creating those page components, rather than passing the home view in as a property, and then calling a method on it.

@arubislander

This comment has been minimized.

Copy link
Author

commented Apr 12, 2019

Thanks for the feedback. Will look into it at the first opportunity.

@arubislander arubislander changed the title Fix https://github.com/ubports/libertine/issues/33 WIP: Fix https://github.com/ubports/libertine/issues/33 Apr 22, 2019

Terence Sambo
* Added packageSelected signal to DebianPackagePicker page and Search…
…ResultsView page

* Changed dynamic instantiation of DebainPackagePicker to static definition because could not get signal handler connection to work with incubator object returned from pageStack.AddToNextColumn()

@arubislander arubislander changed the title WIP: Fix https://github.com/ubports/libertine/issues/33 Fix https://github.com/ubports/libertine/issues/33 Apr 22, 2019

@arubislander

This comment has been minimized.

Copy link
Author

commented Apr 23, 2019

Have added a commit following your pointers and suggestion

@UniversalSuperBox UniversalSuperBox removed this from In progress in OTA-9 Apr 24, 2019

@UniversalSuperBox UniversalSuperBox added this to In progress in OTA-10 via automation Apr 24, 2019

qml/common/DebianPackagePicker.qml Outdated Show resolved Hide resolved
qml/common/DebianPackagePicker.qml Outdated Show resolved Hide resolved
qml/common/SearchResultsView.qml Outdated Show resolved Hide resolved
@dobey

dobey approved these changes May 28, 2019

Copy link
Member

left a comment

Looks OK now. Would be nice to have tests for this stuff too, but it looks like there are no tests for the QML at all yet, so I won't block your changes on that. Thanks!

@dobey dobey merged commit 9957911 into ubports:xenial May 28, 2019

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

OTA-10 automation moved this from In progress to QA May 28, 2019

@UniversalSuperBox UniversalSuperBox removed this from QA in OTA-10 Aug 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.