Community
Participate
Working Groups
If a query-from-Url is used, we could provide an edit-in-WebUI link in the wizard page directing the user to the query editing webpage. This would make it a bit easier to update queries like that.
Great idea Thomas. Should just be a matter of appending a few query parameters to the url to open into the edit query page.
Created attachment 167416 [details] patch v1 a first shot
Created attachment 167417 [details] mylyn/context/zip
Rob, should I do the review and we include this in 3.4?
Created attachment 168292 [details] screenshot I had a quick look. I would expect the edit query button to be disabled when no url is in there (i.e. upon creating a new query). Also, since the dialog is modal, its a little odd when users presses it inevitably thinking they should construct the url using that button. Perhaps the button could be a small one immediately to the right of the url field and only enabled when a valid url is in there, that might help. What do you guys think? Oh, and while we're at it, lets fix the missing ":" on Query URL.
Created attachment 168337 [details] patch v2.1 same old, but with button (de)activation depending on queryText content
Created attachment 168338 [details] mylyn/context/zip
Created attachment 168342 [details] patch v3 Patch with button smaller and next to the queryText. Also does button (de)activation depending on querytext Was that what you meant by smaller button Rob?
Created attachment 168343 [details] mylyn/context/zip
Created attachment 168344 [details] patch v4 alternate approach for button enablement: if the url is not a valid query (not a url or not containing /query.cgi), the edit button opens a new advanced search page. However, I think with that solution we would need an inline browser and not one in the background
Created attachment 168345 [details] mylyn/context/zip
Created attachment 168346 [details] patch v5 Rob, here is a shot at an inlined browser. It's not that bad, maybe we should resize the page? What do you think?
Created attachment 168347 [details] mylyn/context/zip
Created attachment 168647 [details] patch v6 (inline browser) Rob (or Frank), patch 3&4 (comments #8 and #10) are still valid. This patch here is an update to patch 5, for an inlined browser, and still work in progress. If you got a second, take a look at the approach please. I am not sure if I can use the wizard's progress monitor to indicate web page loading, as I think it's done in the UI thread. I might be stuck with my own progress (in that case I should probably use a progress widget, for now it's just a simple label). Thoughts?
Created attachment 168648 [details] mylyn/context/zip
Thomas, I like the implementation with the inlined browser. But what's about to replace the button with an ExpandableComposite. BTW the with is to small to show the page. patch v6 gives me an NPE because ModifyListener is called before connectingLabel is created. I found an other problem. Please change one field in the html page and hit search to see the result. Now the query URL is changed and you never get the Query name even if you fill remember as and hit the button. What I hoped to get is an way that I can change fields of the query or rename the query in the wizard.
(In reply to comment #16) > Thomas, > > I like the implementation with the inlined browser. But what's about to replace > the button with an ExpandableComposite. BTW the with is to small to show the > page. Good idea, I will give that a try. > > patch v6 gives me an NPE because ModifyListener is called before connectingLabel > is created. Ah, thanks. I tested with a new query, so the text is empty and no NPE, but it makes sense that there will be one for an existing query. > > I found an other problem. > > Please change one field in the html page and hit search to see the result. Now > the query URL is changed and you never get the Query name even if you fill > remember as and hit the button. It is intended that as soon as the URL in the browser contains "query.cgi", it will automatically be pasted into the query text field (and the query field will get the incoming color background). Thus no remember as is necessary. It is a good point though that this is not expected by the user and will completely be missed. Maybe I just need a button with something like "use the current query"? > > What I hoped to get is an way that I can change fields of the query or rename > the query in the wizard. I am not sure if we should mix the queries in bugzilla's webUI with the task list queries...for now I would recommend to leave them separated.
Thomas, can you attach a screenshot of the current version?
Created attachment 168996 [details] dialog before pressing edit
Created attachment 168999 [details] dialog after editing a query
Thanks. I'll add that to the UI review for next Thursday.
This is a great idea and works pretty well. A couple of things from the UI review today: * border around browser since the scrollbars are just floating otherwise * move the message about how to set the new url by pressing the search button to the dialog message area instead of the label since it wasn't obvious * can we use the progress bar from wizard since the label seems a bit odd. If not, I think that we should move the progress indication to be under the browser. Comments on the patch: * I got an NPE opening an existing query since the connectingLabel was not yet initialized * all strings should be externalized Also, can we prevent users from navigating to pages other than the search results? (i.e. in changing set e.doit=false)
Thansk for the review. I will most probably not be able to provide another patch before the 3.4 deadline, but how about just applying the minimal changes as posted in patch 3&4 (comments #8 and #10) (without the inlined browser, just the button opening the website)?
I would prefer if you created another patch than we only have to review and debug one implementation.
(In reply to comment #24) > I would prefer if you created another patch than we only have to review and > debug one implementation. And that patch should be with or without the inlined browser? I have not yet heard if this inlined solution is preferred or not. Before working on refining a patch, this decision should be made I think.
Yes, a browser with a location listener is a great feature addition and we agreed on the call that it would be worth trying it even though the inlining of the browser has some drawbacks such as a missing location bar or requiring the user to login in some cases.
ok, great, I will work on refining that. Don't expect it for 3.4 then though :)
Created attachment 169685 [details] patch v7 Alright, had some time in the plane. I picked up Frank's idea about the expandable composite, I like it. I added a progress widget (as I don't have access to the wizard's progress bar), removed the status lable. Worked in my tests, but since I updated Mylyn form CVS today tasks.ui doesn't compile, so I can't test this against the most recent HEAD.
Created attachment 169686 [details] mylyn/context/zip
The query page becomes unusable if an internal browser is not available (e.g. when running 32-bit Eclipse on a 64-bit OS): org.eclipse.swt.SWTError: No more handles [MOZILLA_FIVE_HOME='/usr/lib/xulrunner-addons'] (java.lang.UnsatisfiedLinkError: no swt-mozilla-gtk-3557 or swt-mozilla-gtk in swt.library.path, java.library.path or the jar file) at org.eclipse.swt.SWT.error(SWT.java:3910) at org.eclipse.swt.browser.Mozilla.create(Mozilla.java:516) at org.eclipse.swt.browser.Browser.<init>(Browser.java:119) at org.eclipse.mylyn.internal.bugzilla.ui.tasklist.BugzillaCustomQueryWizardPage.createControl(BugzillaCustomQueryWizardPage.java:155) ...
Created attachment 169749 [details] patch v8 Whoops, thanks for catching that. Not sure how to best resolve this, but since it seems I can't test for a browser without trying to instantiating it, displaiying a link and text inside the expandable composite in case the browser opening fails seems like a quick fix for this.
Created attachment 169750 [details] mylyn/context/zip
Thomas, are you willing to update the patch against the latest and push a review to http://review.mylyn.org so we could consider the enhancement for 3.7?
Steffen, can you elaborate on the process? Do I actually need to create a patch, or just push my changes to http://review.mylyn.org? If so, can you help me do that, explain the process to me? I have yet to use Gerrit.. I applied the changes locally again today, so I would be ready for a first review
I have added documentation in the wiki how to use the Mylyn Gerrit instance: http://wiki.eclipse.org/Mylyn/Contributor_Reference#Reviews
Mylyn has been restructured, and our issue tracking has moved to GitHub [1]. We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub. [1] https://github.com/orgs/eclipse-mylyn
