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

Ignore operator adds wrong context #37

Closed
schroef opened this issue Feb 7, 2018 · 12 comments
Closed

Ignore operator adds wrong context #37

schroef opened this issue Feb 7, 2018 · 12 comments

Comments

@schroef
Copy link
Contributor

schroef commented Feb 7, 2018

v1.0.4

When the Auto-check is in on we see a popup, which is actually already double notification, and then we see the panel with the options. At the bottom it says "X ignore", to my understanding of meaning of ignore. Ignore should ignore the update warning, yet it doesnt. I do see the warning error and that i should check the addon pref. But the menu doesnt go away.

I went on and checked the code. #1284 > addon_updater should be as this:

def ignore_update(self):
	self._json["ignore"] = True
	self.save_updater_json()

When i change this to True, the update panel warning will go away. A tleast i think this how it should have been working. See below screengrab, with example how it is standard and after i edit the code quickly.

autoupdate_warning

@schroef
Copy link
Contributor Author

schroef commented Feb 7, 2018

Perhaps its also better to use "standard" user confirmation setup. So a [CANCEL] button left and an [OK] button right. This makes more sense, also this could/should be added to the popup window.

One small thing, when a user changes the update frequency but prior has set ignore=true. He will never see any update. Perhaps when the time is changed, the ignore function needs to be reset? Or another option is to post pone, this way the checker will be reset giving the user a new warning with the given time.

I think this GUI makes more sense :)
screen shot 2018-02-07 at 4 45 32 am

@TheDuckCow
Copy link
Collaborator

TheDuckCow commented Feb 7, 2018 via email

@schroef
Copy link
Contributor Author

schroef commented Feb 7, 2018

Ahhh yes ofcourse, i know that a wm window only has OK at the bottom, didnt thought about that. But to me the okay seemed like OK will install it, the downside is we cant change the text of that OK button. I just noticed when i press the manual button, which seems also a wm dialog, does have a different text at the bottom.

Also in the popup, there is a part commented;
# could offer to remove popups here, but window will not redraw # so may be confusing to the user/look like a bug # row = layout.row() # row.label("Prevent future popups:") # row.operator(addon_updater_ignore.bl_idname,text="Ignore update")

But when i add;
def check(self, context): return True
to a wm dialog it does update the draw of that dialog, but this ignore should do a cancel as well then.

I put them side by side cause in the render panel that can be done, cycles has a col menu of 3 even.

@TheDuckCow
Copy link
Collaborator

So I see the current dev version for v1.0.5 actually has this UI setup already, somewhat forgot I put this together. Hopefully this makes the popup more reasonable, and then I see that in the v1.0.4 as well as this pre-released v1.0.5 does have the line self._json["ignore"] = True already and behaves in the way in your "corrected" screenshot already. In any event, that line is set to True in v1.0.5 so should be all good.

new popup
I suppose the language could be adjusted slightly, but the behavior should be pretty clear. In case it's not, here's the intent below (and feel free to suggest clearer language to indicate this succinctly):

  • Update now: Will immediately install the update
  • Ignore: It will not update, and it will not show notifications for this update again (but you can still update in user preferences)
  • Defer: It will not update, but it will still show update popups/boxes in future sessions; it also will not make the panel-box go away.

Finally, regarding your point on double notification - you can disable the popup if you feel it's not appropriate for your addon, and that's totally reasonable. Simply edit the background_update_callback function in the addon_updater_ops.py file to return True always, or return before it gets to the code relating to the popup handlers.

I may may this more of an explicit setting in the next release, but in the meantime you can do the above. Let me know if all that is clear!

@schroef
Copy link
Contributor Author

schroef commented Feb 11, 2018

Yes this looks much better doesnt it? PS so now the OK button doesnt install it i think. Also how are you able to set a the button in this panel to a different text? I though a wm dialog always show OK as text.

screen shot 2018-02-11 at 4 17 41 pm

@TheDuckCow
Copy link
Collaborator

Can you link me to the current version of your addon (with the updater code as it is)? The install manually sign generally only comes up if an error occurred. I could take a look and see what the issue is.

Also out of curiosity, are you on a windows machine? You may need to pre-create the updater subdirectory folder as per here.

As for your comment, there are two types of dialogues; one with an OK button at the bottom (with will run the execute function exactly once and dismiss the popup dialogue), and the other type does not put any bottom at the bottom and will re-run the execute function (using the undo stack) whenever you change a property in the draw window (e.g. check/uncheck) - this method does NOT allow you to dismiss a dialogue, the user would have to click away. For the direct download, this seemed okay but generally the preference in my mind is to have an OK button so as to dismiss the dialogue.

@schroef
Copy link
Contributor Author

schroef commented Feb 13, 2018

Can you link me to the current version of your addon (with the updater code as it is)? The install manually sign generally only comes up if an error occurred. I could take a look and see what the issue is.

I havent cleaned up my addon yet and there are some parts which still have errors which need to be fixed prior :) I used the basic addon from cgcookie, v1.0.4

Also out of curiosity, are you on a windows machine? You may need to pre-create the updater subdirectory folder as per here.

Im using OSX and i dont have issues with the structure, i did a quick test and everything was installed properly. Though i need to check if i could exclude files. I noticed that not the release is installed but the main gitHub repo with all folders like wiki, possible images, readme.md and perhaps changelog if there. Is it possible that it installs the release file in stead of the main repo?

As for your comment, there are two types of dialogues; one with an OK button at the bottom (with will run the execute function exactly once and dismiss the popup dialogue), and the other type does not put any bottom at the bottom and will re-run the execute function (using the undo stack) whenever you change a property in the draw window (e.g. check/uncheck) - this method does NOT allow you to dismiss a dialogue, the user would have to click away. For the direct download, this seemed okay but generally the preference in my mind is to have an OK button so as to dismiss the dialogue.

I was just clicking the menu which shows when a updated is available this shows update or ignore, website, manual install. When i clicked manual i noticed it could have custom OK button name. But i think i understand it better now with your explanation

@schroef
Copy link
Contributor Author

schroef commented Feb 13, 2018

` class GithubEngine(object):

def __init__(self):
	self.api_url = 'https://api.github.com'
	self.token = None
	self.name = "github"

def form_repo_url(self, updater):
	return "{}{}{}{}{}".format(self.api_url,"/repos/",updater.user,
							"/",updater.repo)

def form_tags_url(self, updater):
	return "{}{}".format(self.form_repo_url(updater),"/tags")

def form_branch_list_url(self, updater):
	return "{}{}".format(self.form_repo_url(updater),"/branches")

def form_branch_url(self, branch, updater):
	return "{}{}{}".format(self.form_repo_url(updater),
						"/zipball/",branch)

def parse_tags(self, response, updater):
	return response

`
If i change these addresses to /repository/releases/tag/ perhaps it can only download the release zip on not the complete repo with all of it contents. Possibly i would need to edit some code to get it to work. But should be possible.

@schroef
Copy link
Contributor Author

schroef commented Feb 14, 2018

I got it working and will add a fork to repos, shall i do a pull_request or send a link?
This is quite nice, you can install the releases which doesnt have all the other files a repo.

edit
I added a fork, here's the link

@TheDuckCow
Copy link
Collaborator

Hey, this should now be addressed in master and soon will be part of the v1.0.5 release after some delays you are aware of. Let me know if you see any issues with the recently merged Master branch, and if not I'll close the issue.

@schroef
Copy link
Contributor Author

schroef commented Apr 6, 2018

Well i cant really test because of that SSL issues for on my machine :(
Ive only tested the condensed UI and the manual download popup. That all works fine here on OSX 10.13.1 bl 2.78c

PS i know i asked it before, but what code is used for a dialog window without an OK button at the bottom?

@TheDuckCow
Copy link
Collaborator

Tip, you could always test on a virtual machine (e.g. I often use Ubuntu loaded in VirtualBox to test cross compatibility if I'm away from other machines). There, you can test the working code for linux fine.

To get a popup with an OK button: invoke_props_dialog (won't run execute code until OK is pressed)
To get a popup without an OK button: invoke_popup (automatically executes code, and re-executes every time a property of the operator is changed)

Both are setup the same way inside an invoke method of an operator class.

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

No branches or pull requests

2 participants