Skip to content

[FIX] OWBaseLearner: Learner name is changed on output when user changes it and auto apply selected #1453

Merged
lanzagar merged 1 commit intobiolab:masterfrom
PrimozGodec:base_learner
Jul 13, 2016
Merged

[FIX] OWBaseLearner: Learner name is changed on output when user changes it and auto apply selected #1453
lanzagar merged 1 commit intobiolab:masterfrom
PrimozGodec:base_learner

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