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

Issue2 ops browser #15

Merged
merged 9 commits into from
Apr 26, 2016
Merged

Issue2 ops browser #15

merged 9 commits into from
Apr 26, 2016

Conversation

dinkoivanov
Copy link
Contributor

Accumulated changes for the operations browser

@IanMayo
Copy link
Member

IanMayo commented Apr 18, 2016

Will keep open a little longer - awaiting fix to "selective visibility" issue: #11

@IanMayo
Copy link
Member

IanMayo commented Apr 18, 2016

Note: on Linux, I needed to install webkit for in-place browser control to work, as recorded here:
https://www.eclipse.org/forums/index.php/t/235996/

@@ -1,20 +1,20 @@
package samplemodel;
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I can't see what changed in this file. If it was just whitespace due to reformatting, it may have been better to "discard" the changes. Happy to talk this through further with you, if you want.

@IanMayo
Copy link
Member

IanMayo commented Apr 22, 2016

Yes - I think you've got the UI right in this new flow.

We only show the child permutations if the operation is available, and we show the fail message against the operation.

Whilst swimming I'd decided that seemed the best solution - great!

@dinkoivanov
Copy link
Contributor Author

dinkoivanov commented Apr 24, 2016

I did some refactoring to cleanup the operation builders. Now the SampleModelOperationRegistry is only responsible to read the extensions (plugin.xml). The buildLibrary method and all related logic (isApplicable, getInputPermutations) is located in a base builder class OperationsLibraryBuilder, with two subclasses for the tree and menu. I think this looks simpler, but let me know if there could still be improvements.
Also let me know if there're some other changes needed to merge the branch.

@IanMayo IanMayo merged commit 0133fc3 into master Apr 26, 2016
@IanMayo IanMayo mentioned this pull request Apr 27, 2016
@IanMayo IanMayo deleted the issue2_ops_browser branch November 2, 2016 12:34
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