-
Notifications
You must be signed in to change notification settings - Fork 51
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
Errors when trying to load dialogs/sliders #89
Comments
created git repository that can simulate problem: https://github.com/sdutry/issue89 start with then go to http://localhost:8080/issue89/issue89.action I'm not completely confident that it's not just a misconfiguration since the showcase site does work. |
Got the same issue with autocompleter. Seems like core.js (jquery ui) is not loaded. Using loadAtOnce=true in the head tag should fix the problem. Maybe the showcase is configured this way. |
@flofourcade loading too late of one of the libraries could be the problem
The fact that none of the specific items actualy get executed right away on the showcase might be the reason why it works. doesn't look like the showcase is configured with the
|
Ok.Saw that head.ftl has been changed. I ll take a look tomorrow . |
@flofourcade |
loaded from google:
not loaded from google
so unless i'm mistaken, it's missing the following javascripts:
|
Great. |
further testing shows that without the from google:
not from google:
I think it might be nescessary to always load the |
@matorodriguez it seems like the google urls don't contain the i18n folder with the different translations for version It did for all previous versions, so the only current workaround option you have is to add |
Great work @sdutry, thanks a lot! |
@jogep @flofourcade The problem i have is that the moment i made the branch off of |
The problem with always loading jquery-ui.js is that we loose the "lazy" loading of jquery-uis's component when loadAtOnce and loadFromGoogle are false. When false, the expected behaviour is that each widget use self.require to load each needed jquery-ui's component using the splitted version of jquery ui js (selectable.js, core.js, slider.js,...). Between the last snapshot and the release, this splitted version of jquery ui has changed (new unique-id.js, some functions have been moved to other file,...) . I think the only way to restore the expected behaviour of loadAtOnce="false" is to test every widget individually and to fix broken dependencies with self.require. |
@flofourcade I can try to have a look at it this weekend, working my way through them one at a time. ps: any idea how to get around the 2 commits ahead the moment i make a fork? |
Sorry, I'm far to be a github's specialist ! Maybe is there a way to exclude some file when you'll create your PR ? |
Use The best option to prepare PRs is to clone the original repo locally, then make a fork on GitHub, add it as a
now when you're at
make changes and commit, now push to your
go to GitHub and you will see that you can create a PR, open it and you're done :) When your PR went in, update
and you can start creating a new PR :) |
@lukaszlenart
So i don't know where those commits originate from edit: |
@flofourcade see commit I'll continue going through the rest 1 by 1, i just wanted to know if i was heading the right way with this fix. |
@sdutry |
@sdutry |
@flofourcade I was still juggling with the javascripts to figure it out. |
Ok, I've tested dialogs-*.jsp of the showcase in isolation (loading it without ajax and with a sj:head without loadAtOnce or loadByGoogle). |
finaly found why it had to be first: inside
|
The sad thing is that we have to test all tags in isolation, it's highly probable that other tags are impacted ! |
i'll start by going around and fixing the location of widget.js wherever mouse.js is loaded. |
seems that one of the main problems for some of the javascripts like
would be that they don't have putting a javascript infront of them that has the following ui dummy definition if it doesn't exist, fixes it: maybe it would be a better idea to put the or would it be an option to modify the files with the problems to add the code below to them? |
@sdutry self.require([
"js/base/widget" + self.minSuffix + ".js",
"js/base/version" + self.minSuffix + ".js",
"js/base/unique-id" + self.minSuffix + ".js",
"js/base/position" + self.minSuffix + ".js",
"js/base/menu" + self.minSuffix + ".js",
"js/base/keycode" + self.minSuffix + ".js",
"js/base/safe-active-element" + self.minSuffix + ".js",
"js/base/autocomplete" + self.minSuffix + ".js"
]); and jquery.ui.struts:.js line 1050 self.require([ "js/base/widget" + self.minSuffix + ".js", "js/base/button" + self.minSuffix + ".js","js/base/tooltip" + self.minSuffix + ".js" ]); Needed to add unique-id.js and tooltip.js for combobox and to change depencies loading order. |
@flofourcade i just did that and commited it, although my ordering of requires is a bit different edit: i noticed your self.require adjustment for autocomplete-selectboxes and updated code accordingly. |
Yes you're right for keycode and safe-active-element, got the same problem while trying to fix autocomplete. We can try your solution based on version.js I think it's a good way. |
which solution then?
|
Add version.js to head.ftl. you can declare it in place of the commented declaration of core.js which is deprecated now. With this setup, version.js will be loaded when in lazy mode |
@flofourcade |
additional problem:
(will fix shortly, just putting it here so i don't forget) Edit: fixed with this commit |
@sdutry no idea what's wrong, I'm always fetch & pull the original repo before creating a branch and it works like a charm. |
can you check something for me: when trying to load the the problem seems to be that the following javascript is executed before the table is loaded and thus doesn't get the chance to bind the click on the rows.
placing those lines inside a document ready is what fixes it for me, but i'm not sure if it's something that should be addressed or if it's only a local problem for me. (or if this is the preferred workaround/solution) |
Ok will have a look a this ASAP. |
checked tags (against branch issue89fix )(will be updated):
broken:
(moved comment to the bottom to keep track of what's checked) |
Considering the branch i'm working on for this issue has a lot of changes already, i was thinking about making a pull request for what's fixed and moving the other remaining (mostly minor) problems into their own issues. I'd like to hear your opinion on this.
please let me know, so i could fix them before making the pull-request. link to branch: issue89fix |
Since the snapshot 20161124.181023-12 (including release 4.0.1 too), I'm getting errors when trying to load dialogs/sliders.
I used the following browsers:
Chrome: 54.0.2840.99 m (64-bit)
Firefox: 50.0.2
DIALOG
Example:
SLIDER
Example:
Latest snapshot working version: 4.0.1-20160922.161421-11, but I have just realized that version 4.0.1-SNAPSHOT was removed from snapshot's repository.
Thanks!
The text was updated successfully, but these errors were encountered: