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] OWBaseLearner: Learner name is changed on output when user changes it and auto apply selected #1453

Merged
merged 1 commit into from
Jul 13, 2016

Conversation

PrimozGodec
Copy link
Contributor

Learner name field didn't have callback when it is changed. Problem was when user changed name and auto apply was on, because name haven't changed on output. It is fixed with this PR.

@codecov-io
Copy link

codecov-io commented Jul 13, 2016

Current coverage is 87.91%

Merging #1453 into master will not change coverage

@@             master      #1453   diff @@
==========================================
  Files            77         77          
  Lines          7582       7582          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           6666       6666          
  Misses          916        916          
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by d0d9b84...1008ae0

@lanzagar
Copy link
Contributor

Now it sends a new output after every key press. This can make it really slow when connected to e.g. Test&Score.
I think in other places we apply the change on focus-out only (or when pressing enter after editing)?

@PrimozGodec
Copy link
Contributor Author

PrimozGodec commented Jul 13, 2016

@lanzagar I changed it that now apply happens only on focus-out. One thing that I do not like too much is button that shows that field is updated but not applied. It is part of gui script.
screenshot from 2016-07-13 13-38-22

@lanzagar
Copy link
Contributor

It might be acceptable if the button appeared to the right of the text field.
Definitely not above, as in the screenshot :)

@PrimozGodec
Copy link
Contributor Author

It has been fixed

@lanzagar
Copy link
Contributor

Looks good to me now.
You can squash all commits into the first and I'll merge.

@PrimozGodec
Copy link
Contributor Author

Done

@lanzagar lanzagar merged commit e57cbeb into biolab:master Jul 13, 2016
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.

3 participants