Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: SVN trunk
    • Component/s: ALL APPLICATIONS
    • Labels:
      None

      Description

      Following Sascha Rodekamp's work on layer lookups OFBIZ-3374 and improvements OFBIZ-3430, I propose now to replace old the popup lookups by layered (Ajaxified) lookups.

      For that please find a patch attached. In this patch I followed a simple S/R tactic:

      • I replaced all occurences of LookupDecorator by LookupLayerPopupDecorator in screens
      • I replaced all occurences of <lookup by <lookup presentation="layer" position="center"

      It's as simple as this. For the moment I decided to use as default position="center" because it's was the easiest (sure that any lookups will be out of the screen). I think we will refine this by removing position="center" and use the default (position="normal") which does not move the layer from the point it's called and will be more aesthetic.

      I did not test anything in OFBIz OOTB for the moment, but I already use layered lookups in a custom application without any issues so far.

      The only drawback I found for the moment is when a lookup is called from a lookup. If you are aware of such cases please chime in.

      Of course everybody is encouraged to test this improvement as much as possible. I really think it's a very cool feature for users, and they will appreciate. There are still some ideas like that (see the link Sasca referred to in OFBIZ-3374), and we will try to implement them.

      There are issues to be fixed prior to a commit, see OFBIZ-3446.

      1. ASF.LICENSE.NOT.GRANTED--OFBIZ-3442 replace popup lookups by layered lookups.patch
        9 kB
        Jacques Le Roux
      2. OFBIZ-3442 replace popup lookups by layered lookups.patch
        9 kB
        Jacques Le Roux
      3. LayoutFixes.patch
        3 kB
        Sascha Rodekamp
      4. LayoutFixes.patch
        4 kB
        Sascha Rodekamp
      5. LayoutFixMinMaxTomah.patch
        0.4 kB
        Sascha Rodekamp
      6. Min-max issue from FTL.jpg
        31 kB
        Jacques Le Roux

        Issue Links

          Activity

          Hide
          Sascha Rodekamp added a comment -

          ui quiet a big one :-D.

          I will have a look.

          Show
          Sascha Rodekamp added a comment - ui quiet a big one :-D. I will have a look.
          Hide
          Jacques Le Roux added a comment -

          Previous patch version wa removing the std LookupDecorator. We still need it of course.
          Notably because for now we can't call a layered lookup from a layered lookup.

          Show
          Jacques Le Roux added a comment - Previous patch version wa removing the std LookupDecorator. We still need it of course. Notably because for now we can't call a layered lookup from a layered lookup.
          Hide
          Jacques Le Roux added a comment -

          New patch

          • includes flt files. For syntactic reasons these lokups will not be centered by default but at "normal" position
          • remove previously layerer lookups which are actually lists (locales, timezones and themes) I have commited a change for that at r907310 (renamed them List instead of Lookup to avoid confusion)
          Show
          Jacques Le Roux added a comment - New patch includes flt files. For syntactic reasons these lokups will not be centered by default but at "normal" position remove previously layerer lookups which are actually lists (locales, timezones and themes) I have commited a change for that at r907310 (renamed them List instead of Lookup to avoid confusion)
          Hide
          Jacques Le Roux added a comment -

          More to come, crossed many small bugs, any from the layering lookup effort so far... I think I will commit soon...

          Show
          Jacques Le Roux added a comment - More to come, crossed many small bugs, any from the layering lookup effort so far... I think I will commit soon...
          Hide
          Sascha Rodekamp added a comment -

          hi Jacques,
          i had a look to your patch.
          The Accounting Module works fine with the new lookups.
          But by replacing the decorator screens in the common modul, some other (old) lookups looks quite ugly now (They uses the new decorator which didn't load the nessary css files for the (old) lookups). You can see this i.e. in the catalog manager. https://localhost:8443/catalog/control/main.

          It's not a big deal, i think, because it's only for the interim period.

          Have a nice day (WE)

          Sascha

          Show
          Sascha Rodekamp added a comment - hi Jacques, i had a look to your patch. The Accounting Module works fine with the new lookups. But by replacing the decorator screens in the common modul, some other (old) lookups looks quite ugly now (They uses the new decorator which didn't load the nessary css files for the (old) lookups). You can see this i.e. in the catalog manager. https://localhost:8443/catalog/control/main . It's not a big deal, i think, because it's only for the interim period. Have a nice day (WE) Sascha
          Hide
          Sascha Rodekamp added a comment -

          The same issue occurs if you try to change your visual theme or language.

          The Screen "ListVisualThemes" loads the LookupLayerPopupDecorator, but should load the LookupDecorator.

          Show
          Sascha Rodekamp added a comment - The same issue occurs if you try to change your visual theme or language. The Screen "ListVisualThemes" loads the LookupLayerPopupDecorator, but should load the LookupDecorator.
          Hide
          Jacques Le Roux added a comment -

          Hi Sascha,

          Did you try with a fresh checkout (without prior layer related changes) using the last patch? Because, as you read in my comment above I did take care of the ListVisualThemes like issues (and it works well on my machine, though I did not find how to test the time zones case).

          What do you mean exactly by looking ugly, with which themes did you try? I used mostly Dropping Crumb for my tests and appart issues with LookupTreeContent and LookupDetailContentTree, all seems to work correctly here.

          So it's almost done now and I will commit as soon as I wil have resolved the 2 issues above (I'm struggling with them from a moment and just go round in circles for now)

          Thanks for your help.

          Show
          Jacques Le Roux added a comment - Hi Sascha, Did you try with a fresh checkout (without prior layer related changes) using the last patch? Because, as you read in my comment above I did take care of the ListVisualThemes like issues (and it works well on my machine, though I did not find how to test the time zones case). What do you mean exactly by looking ugly, with which themes did you try? I used mostly Dropping Crumb for my tests and appart issues with LookupTreeContent and LookupDetailContentTree, all seems to work correctly here. So it's almost done now and I will commit as soon as I wil have resolved the 2 issues above (I'm struggling with them from a moment and just go round in circles for now) Thanks for your help.
          Hide
          Jacques Le Roux added a comment -

          Forgot to say that I tested almost all screens...Only Sales app has to be reviewed, I will do soon...

          Show
          Jacques Le Roux added a comment - Forgot to say that I tested almost all screens...Only Sales app has to be reviewed, I will do soon...
          Hide
          Sascha Rodekamp added a comment -

          hm jep jacques i performed an update before i tested your patch.
          I looked in the common LookupScreens and the visualTheme Screen calls the LookupLayerPopupDecorator.
          The line 5104 in your last patch you cahnage the Decorator .

          I looked in Theme BizzTime and BlueLight

          Show
          Sascha Rodekamp added a comment - hm jep jacques i performed an update before i tested your patch. I looked in the common LookupScreens and the visualTheme Screen calls the LookupLayerPopupDecorator. The line 5104 in your last patch you cahnage the Decorator . I looked in Theme BizzTime and BlueLight
          Hide
          Jacques Le Roux added a comment - - edited

          SFA looks good but some not related bugs I found, also in Party an Project.

          I think a week more is needed to clarify all that...

          Show
          Jacques Le Roux added a comment - - edited SFA looks good but some not related bugs I found, also in Party an Project. I think a week more is needed to clarify all that...
          Hide
          Bilgin Ibryam added a comment -

          Hi Jacques and Sascha, this a great addition, thanks a lot for working on it.

          I did some quick tests. Here is what I found.

          Before few weeks I added autocompleter support to lookup fields. (If you didn't see it, just type one/two letters in a lookup field and you can select an entry w/o using the lookup button). It was missing in LookupLayerPopupDecorator and I added it in r907456. Now autocompleter and layered lookups work together fine

          I didn't follow all the development process of the layered lookups, but why not change the default values in the schema of presentation and position elements to layer and center respectively. Then we don't need to add these attributes in all the forms.

          Also I found a bug caused when user session is destroyed: Log into an application (accounting for example) and select a form - layered lookups work correct. In another tab log into another application and then log out. Then on the first tab (accounting) try to use a lookup. In the lookup screen you will get the login screen (that's OK) but once you enter your username and password you get a blank screen.

          Bilgin

          Show
          Bilgin Ibryam added a comment - Hi Jacques and Sascha, this a great addition, thanks a lot for working on it. I did some quick tests. Here is what I found. Before few weeks I added autocompleter support to lookup fields. (If you didn't see it, just type one/two letters in a lookup field and you can select an entry w/o using the lookup button). It was missing in LookupLayerPopupDecorator and I added it in r907456. Now autocompleter and layered lookups work together fine I didn't follow all the development process of the layered lookups, but why not change the default values in the schema of presentation and position elements to layer and center respectively. Then we don't need to add these attributes in all the forms. Also I found a bug caused when user session is destroyed: Log into an application (accounting for example) and select a form - layered lookups work correct. In another tab log into another application and then log out. Then on the first tab (accounting) try to use a lookup. In the lookup screen you will get the login screen (that's OK) but once you enter your username and password you get a blank screen. Bilgin
          Hide
          Jacques Le Roux added a comment -

          Hi Bilgin,

          Thanks for your help on autocomplete, it's great to see a community in action

          We need to keep the LookupDecorator, but yes not sure it should be the default now (after a deep testing session this weekend). We need to discuss that on dev ML though, to be sure that a large consensus is reached and we don't miss something before making layer the default presentation.

          Maybe having center as default position is also a good idea, for instance it's where dialog boxes (Windows) and such open on most OS. I think we need to discuss this as well on dev ML...

          I already opened an issue (sub-task acutally OFBIZ-3450)for the case you detailled, I will put your details, thanks!

          Show
          Jacques Le Roux added a comment - Hi Bilgin, Thanks for your help on autocomplete, it's great to see a community in action We need to keep the LookupDecorator, but yes not sure it should be the default now (after a deep testing session this weekend). We need to discuss that on dev ML though, to be sure that a large consensus is reached and we don't miss something before making layer the default presentation. Maybe having center as default position is also a good idea, for instance it's where dialog boxes (Windows) and such open on most OS. I think we need to discuss this as well on dev ML... I already opened an issue (sub-task acutally OFBIZ-3450 )for the case you detailled, I will put your details, thanks!
          Hide
          Sascha Rodekamp added a comment -

          Hi Jacques,

          i have a small update for your patch. Fixed the layout problem with the choose visual theme and choose language lookups.

          Best regards
          Sascha

          Show
          Sascha Rodekamp added a comment - Hi Jacques, i have a small update for your patch. Fixed the layout problem with the choose visual theme and choose language lookups. Best regards Sascha
          Hide
          Jacques Le Roux added a comment -

          Hi Sascha,

          Yes I had fixed that already. As I use the layered lookups in a custom application I had to fix it there. I fixed also the ListTimezones but at this time I did not create a patch as I thought I would commit all the changes. Actually it's ready to be commited for almost a month now and I think I will really commit pretty soon. I think I will let apart the lookups which are not working (with or without layering) to not let think we introduced bugs with the layering... Except if Iget enough time to fix them...

          Show
          Jacques Le Roux added a comment - Hi Sascha, Yes I had fixed that already. As I use the layered lookups in a custom application I had to fix it there. I fixed also the ListTimezones but at this time I did not create a patch as I thought I would commit all the changes. Actually it's ready to be commited for almost a month now and I think I will really commit pretty soon. I think I will let apart the lookups which are not working (with or without layering) to not let think we introduced bugs with the layering... Except if Iget enough time to fix them...
          Hide
          Sascha Rodekamp added a comment -

          Hi Jacques,
          that sounds great... so if there anythink where i can help you... let me know! I think we can handle this

          Show
          Sascha Rodekamp added a comment - Hi Jacques, that sounds great... so if there anythink where i can help you... let me know! I think we can handle this
          Hide
          Jacques Le Roux added a comment -

          Also, though I agree with Bilgin on the defaults to use, I think I will commit as is for now. For this 2 reasons:

          1. It's easy to change later with a S/R and small changes in XSD
          2. I'm not quite sure we should use these defaults without exchanging before with the community. However so far, we are few to be really interested (or simply aware), but I guess after it will be commited a lot of discussion will occurs and maybe new ideas, etc.
          Show
          Jacques Le Roux added a comment - Also, though I agree with Bilgin on the defaults to use, I think I will commit as is for now. For this 2 reasons: It's easy to change later with a S/R and small changes in XSD I'm not quite sure we should use these defaults without exchanging before with the community. However so far, we are few to be really interested (or simply aware), but I guess after it will be commited a lot of discussion will occurs and maybe new ideas, etc.
          Hide
          Jacques Le Roux added a comment -

          Last patch and last chance to test before commit

          Show
          Jacques Le Roux added a comment - Last patch and last chance to test before commit
          Hide
          Jacques Le Roux added a comment -

          Please don't use the patches here they are buggy and I should hopefuly post another tomorrow which should fix some issues I found (not due to layered lookup feature itself but its limitations, for instance you can't call a lookup from a lookup yet: OFBIZ-3446)

          Show
          Jacques Le Roux added a comment - Please don't use the patches here they are buggy and I should hopefuly post another tomorrow which should fix some issues I found (not due to layered lookup feature itself but its limitations, for instance you can't call a lookup from a lookup yet: OFBIZ-3446 )
          Hide
          Jacques Le Roux added a comment -

          To bypass the lookup in lookup issue, in a 1s time I will use the bad way (easier for now). Then I will create the extends+extends-resource mechanims for lookup, will use it and remove then the unneeded entries in controllers. Another better way would be to "fix" the lookup in lookup. But I prefer to do that later...

          Show
          Jacques Le Roux added a comment - To bypass the lookup in lookup issue, in a 1s time I will use the bad way (easier for now). Then I will create the extends+extends-resource mechanims for lookup, will use it and remove then the unneeded entries in controllers. Another better way would be to "fix" the lookup in lookup. But I prefer to do that later...
          Hide
          Jacques Le Roux added a comment - - edited

          A patch without the ftl changes and without the lookups calling a lookup inside. We still need to make the changes in FTL file and test OFBIZ-3446 thoroughly and the layered lookups will be done (but 2 not blocking small issues, see the umbrella task)

          Note that the effort for lookup in ftl files is a part of OFBIZ-3541 and is well explained here

          Show
          Jacques Le Roux added a comment - - edited A patch without the ftl changes and without the lookups calling a lookup inside. We still need to make the changes in FTL file and test OFBIZ-3446 thoroughly and the layered lookups will be done (but 2 not blocking small issues, see the umbrella task) Note that the effort for lookup in ftl files is a part of OFBIZ-3541 and is well explained here
          Hide
          Jacques Le Roux added a comment - - edited

          There is now an incomplete patch for flt files at OFBIZ-3541

          I wonder if I will not create a branch for this.

          Show
          Jacques Le Roux added a comment - - edited There is now an incomplete patch for flt files at OFBIZ-3541 I wonder if I will not create a branch for this.
          Hide
          Jacques Le Roux added a comment -

          Sascha, Bilgin,

          I thought a bit more about the default position. Actually Sascha followed how the calendar was working and created the "normal" postion as default. We talked about making center the default, but I think we should keep the position used previously but the lookups: topleft. Not much because it was there before, but rather because it's the better position when you need to enlarge the lookup. There is only one thing to do: drag the right bottom corner. It's easier than in any other position. So will finally make it the default. I will also rename "normal" to "near".

          Show
          Jacques Le Roux added a comment - Sascha, Bilgin, I thought a bit more about the default position. Actually Sascha followed how the calendar was working and created the "normal" postion as default. We talked about making center the default, but I think we should keep the position used previously but the lookups: topleft. Not much because it was there before, but rather because it's the better position when you need to enlarge the lookup. There is only one thing to do: drag the right bottom corner. It's easier than in any other position. So will finally make it the default. I will also rename "normal" to "near".
          Hide
          Sascha Rodekamp added a comment -

          Jep Jacques, thats a good point and shouldn't be a problem to change.

          Show
          Sascha Rodekamp added a comment - Jep Jacques, thats a good point and shouldn't be a problem to change.
          Hide
          Bilgin Ibryam added a comment -

          Jacques,

          what I tried to say is that the default position (it doesn't matter for me if it is center , normal or something else) should be set only in xsd file, but not in all the forms and fields. This way, changing the default position will require only a change in xsd file instead of all the form fields.

          Show
          Bilgin Ibryam added a comment - Jacques, what I tried to say is that the default position (it doesn't matter for me if it is center , normal or something else) should be set only in xsd file, but not in all the forms and fields. This way, changing the default position will require only a change in xsd file instead of all the form fields.
          Hide
          Jacques Le Roux added a comment -

          Yes sure Bilgin,

          I totally agree, that's what I will do, as I said For the moment I did not change in the patch because I was not sure of the default position. But, like you suggested, it will be at end without layer= and position=

          Show
          Jacques Le Roux added a comment - Yes sure Bilgin, I totally agree, that's what I will do, as I said For the moment I did not change in the patch because I was not sure of the default position. But, like you suggested, it will be at end without layer= and position=
          Hide
          Jacques Le Roux added a comment - - edited

          This is the last patch with the minium modifications needed to replace all popup lookups by layered lookup. I have already identified some cases where we want not to use layered lookups. LookupTimeDuration is one there are maybe others but I wonder at this stage if I will get enough support from this issue or I will not rather commit as is and wait returns to fix issues as it's realy easy to fix since a simple <<presentation="window">> attribute does the trick

          As I said there are still some issues though, notably with

          1. the last lookup (at bottom) of https://localhost:8443/workeffort/control/EditWorkEffort?workEffortTypeId=TASK&currentStatusId=CAL_NEEDS_ACTION : look for ProductId inside. This is something specific to layer in layer and we have already OFBIZ-3446 opened for that
          2. same with https://localhost:8443/content/control/FindSurvey (acroform)
          3. also https://localhost:8443/assetmaint/control/LookupWorkEffort (recursive issue: look for parent) we have already OFBIZ-3693 opened for that
          4. The minimize/enlarge buttons (+ -) blocks the layer see OFBIZ-3692
          5. I saw still some problems in themes other than Tomahawk

          According to Ankit Jain (see OFBIZ-3446), it seems that when you use the attribute <<presentation="layer">> attribute some of these issues does not exist. I have also noticed some differences but at this stage I'm not quite sure. I will revisit this ASAP but maybe not today.

          I'd really appreciate more testing, thanks for you help

          Show
          Jacques Le Roux added a comment - - edited This is the last patch with the minium modifications needed to replace all popup lookups by layered lookup. I have already identified some cases where we want not to use layered lookups. LookupTimeDuration is one there are maybe others but I wonder at this stage if I will get enough support from this issue or I will not rather commit as is and wait returns to fix issues as it's realy easy to fix since a simple <<presentation="window">> attribute does the trick As I said there are still some issues though, notably with the last lookup (at bottom) of https://localhost:8443/workeffort/control/EditWorkEffort?workEffortTypeId=TASK&currentStatusId=CAL_NEEDS_ACTION : look for ProductId inside. This is something specific to layer in layer and we have already OFBIZ-3446 opened for that same with https://localhost:8443/content/control/FindSurvey (acroform) also https://localhost:8443/assetmaint/control/LookupWorkEffort (recursive issue: look for parent) we have already OFBIZ-3693 opened for that The minimize/enlarge buttons (+ -) blocks the layer see OFBIZ-3692 I saw still some problems in themes other than Tomahawk According to Ankit Jain (see OFBIZ-3446 ), it seems that when you use the attribute <<presentation="layer">> attribute some of these issues does not exist. I have also noticed some differences but at this stage I'm not quite sure. I will revisit this ASAP but maybe not today. I'd really appreciate more testing, thanks for you help
          Hide
          Sascha Rodekamp added a comment -

          Hi Jacques,

          sry for mmy late response. Good idea with the min. modifications. I tested your patch and i see one issue. You changed the call method but not the lookups itself. Remember we set an extra decorator for the new lookups because of the different behavior (LookupLayerPopupDecorator). That causes some issus when i try to open a layer form a layer (tested in Workeffort). I think the easiest way to fix this is to make the new decorator default (rename in "LookupLayerDecorator" and the old LookupLayerDecorator in "LookupLayerWindowDecorator") then you have to do only a few changes in the decorators you want to open in the classic way.

          What do you think about it??

          Have a nice day

          Sascha

          Show
          Sascha Rodekamp added a comment - Hi Jacques, sry for mmy late response. Good idea with the min. modifications. I tested your patch and i see one issue. You changed the call method but not the lookups itself. Remember we set an extra decorator for the new lookups because of the different behavior (LookupLayerPopupDecorator). That causes some issus when i try to open a layer form a layer (tested in Workeffort). I think the easiest way to fix this is to make the new decorator default (rename in "LookupLayerDecorator" and the old LookupLayerDecorator in "LookupLayerWindowDecorator") then you have to do only a few changes in the decorators you want to open in the classic way. What do you think about it?? Have a nice day Sascha
          Hide
          Jacques Le Roux added a comment -

          Hi Sascha,

          Yes I wondered about that, but as I got the same kind of issues (as you layer in layer, findsurvery and workeffort as well) with or without them I took this way for testing (I should have look into code but did not have time at this moment). I have attached a kind of diff file (lookup.html) and apart collapsible="true" ws "false" I'm not quite sure there are much needs to change... Are the snippets you removed issues in the layer (notably including ftl templates)?

          Ha, also I thought that we would like to expose both cases in example (layer and window) but this is easy, just use <<presentation="window">>

          Show
          Jacques Le Roux added a comment - Hi Sascha, Yes I wondered about that, but as I got the same kind of issues (as you layer in layer, findsurvery and workeffort as well) with or without them I took this way for testing (I should have look into code but did not have time at this moment). I have attached a kind of diff file (lookup.html) and apart collapsible="true" ws "false" I'm not quite sure there are much needs to change... Are the snippets you removed issues in the layer (notably including ftl templates)? Ha, also I thought that we would like to expose both cases in example (layer and window) but this is easy, just use <<presentation="window">>
          Hide
          Sascha Rodekamp added a comment -

          The Main part to remove in the new decorator was the header and footer including, they are not needed in the new lookup.
          Also shouldn't be the lookup.ftl loaded in the layer lookup because it causes some site effects with the set value function.

          Show
          Sascha Rodekamp added a comment - The Main part to remove in the new decorator was the header and footer including, they are not needed in the new lookup. Also shouldn't be the lookup.ftl loaded in the layer lookup because it causes some site effects with the set value function.
          Hide
          Jacques Le Roux added a comment -

          I see, If we could pass a parameter like it's done for parameters.ajaxLookup then we could handle that locally with fail-widget. I'd prefer to have only one decorator as they are almost the same. For instance a block was recentlu added by Erwan at r897843 and miss in the new layer decorator.

          BTW I believe there is still an issue with ajaxLookup when the lookup is called from an FTL file. We could fix the 2 in a single shoot...

          Show
          Jacques Le Roux added a comment - I see, If we could pass a parameter like it's done for parameters.ajaxLookup then we could handle that locally with fail-widget. I'd prefer to have only one decorator as they are almost the same. For instance a block was recentlu added by Erwan at r897843 and miss in the new layer decorator. BTW I believe there is still an issue with ajaxLookup when the lookup is called from an FTL file. We could fix the 2 in a single shoot...
          Hide
          Sascha Rodekamp added a comment -

          jap Jacques good point. We should do it!

          Show
          Sascha Rodekamp added a comment - jap Jacques good point. We should do it!
          Hide
          Jacques Le Roux added a comment -

          Bilgin,

          Could you have a look at the parameters.ajaxLookup thing when you get a chance? I'm maybe wrong or missing something, but it looks to me like it's not working from FTL files. Then when we will have this mechanism from both side (FTL and forms) we will use it as I suggested above to differentiate lookups types (layer vs window) and we should be done (but some minor issues still crawling around)

          TIA

          Show
          Jacques Le Roux added a comment - Bilgin, Could you have a look at the parameters.ajaxLookup thing when you get a chance? I'm maybe wrong or missing something, but it looks to me like it's not working from FTL files. Then when we will have this mechanism from both side (FTL and forms) we will use it as I suggested above to differentiate lookups types (layer vs window) and we should be done (but some minor issues still crawling around) TIA
          Hide
          Jacques Le Roux added a comment -

          Hi Bilgin,

          Forget what I said, I saw the ajaxLookup working in an FTL field (Customer field in the 1st Order Entry screen in Order Manager). It's weird in Firefox 3.6.3 because some time it works some time not (but with my 58 plugins ;o). It's ok in Opera. I don't remember if I have not opened a Jira issue for that (I don't this so), maybe I simply wrote something about that in another place.

          Show
          Jacques Le Roux added a comment - Hi Bilgin, Forget what I said, I saw the ajaxLookup working in an FTL field (Customer field in the 1st Order Entry screen in Order Manager). It's weird in Firefox 3.6.3 because some time it works some time not (but with my 58 plugins ;o). It's ok in Opera. I don't remember if I have not opened a Jira issue for that (I don't this so), maybe I simply wrote something about that in another place.
          Hide
          Sascha Rodekamp added a comment -

          Hi Jacques,

          i removed the second Decorator. Now your previous patch should work with the default lookupDecorator.

          Cheers

          Sascha

          Show
          Sascha Rodekamp added a comment - Hi Jacques, i removed the second Decorator. Now your previous patch should work with the default lookupDecorator. Cheers Sascha
          Hide
          Jacques Le Roux added a comment -

          Thanks Sascha,

          Your slightly modified patch is in trunk at r935578 but the the FormWidgetExampleForms.xml change: I want to keep an old style lookup to allow the comparaison after the change

          Show
          Jacques Le Roux added a comment - Thanks Sascha, Your slightly modified patch is in trunk at r935578 but the the FormWidgetExampleForms.xml change: I want to keep an old style lookup to allow the comparaison after the change
          Hide
          Sascha Rodekamp added a comment -

          Hi Jacques,

          jep thanks. OK i was wondering, why the first lookup up is an old style lookup :-D, thats why i changed it... So no problem

          Show
          Sascha Rodekamp added a comment - Hi Jacques, jep thanks. OK i was wondering, why the first lookup up is an old style lookup :-D, thats why i changed it... So no problem
          Hide
          Jacques Le Roux added a comment -

          I tested with Flat Grey and Tomahawk

          Show
          Jacques Le Roux added a comment - I tested with Flat Grey and Tomahawk
          Hide
          Jacques Le Roux added a comment -

          BTW from the change in your patch I wonder if it's not another commit which has changed things. Fortunately I have another instance to test, right now...

          Show
          Jacques Le Roux added a comment - BTW from the change in your patch I wonder if it's not another commit which has changed things. Fortunately I have another instance to test, right now...
          Hide
          Jacques Le Roux added a comment -

          Confirmation, another commit (related to Blas's patches ?) has broken the inside buttons...

          Show
          Jacques Le Roux added a comment - Confirmation, another commit (related to Blas's patches ?) has broken the inside buttons...
          Hide
          Sascha Rodekamp added a comment -

          Jep i noticed this issue, the pagination buttons are broken too. Hm i don't know what happened.

          Show
          Sascha Rodekamp added a comment - Jep i noticed this issue, the pagination buttons are broken too. Hm i don't know what happened.
          Hide
          Jacques Le Roux added a comment -

          Maybe r935312...

          Show
          Jacques Le Roux added a comment - Maybe r935312...
          Hide
          Sascha Rodekamp added a comment -

          Jep that might be the problem, i will have look tomorrow morning. And try to figure out what happened.

          Show
          Sascha Rodekamp added a comment - Jep that might be the problem, i will have look tomorrow morning. And try to figure out what happened.
          Hide
          Jacques Le Roux added a comment -

          I tried to revert, and, after some necessary changes here and there, I believe we should better handle the new situation...

          Show
          Jacques Le Roux added a comment - I tried to revert, and, after some necessary changes here and there, I believe we should better handle the new situation...
          Hide
          Jacques Le Roux added a comment -

          I have no time right now but ASAP I will update the patch because It's not good anymore (because of changes in htmlFormMacroLibrary.ftl)

          Show
          Jacques Le Roux added a comment - I have no time right now but ASAP I will update the patch because It's not good anymore (because of changes in htmlFormMacroLibrary.ftl)
          Hide
          Jacques Le Roux added a comment -

          New patch, finally was not a big deal

          Show
          Jacques Le Roux added a comment - New patch, finally was not a big deal
          Hide
          Jacques Le Roux added a comment -

          Keep things simple

          Show
          Jacques Le Roux added a comment - Keep things simple
          Hide
          Sascha Rodekamp added a comment -

          Layout Fix for the issue you posted above.

          The min/max button was shown incorrect and the lookup image in a lookup wasen't displayed.

          So long

          Sascha

          Show
          Sascha Rodekamp added a comment - Layout Fix for the issue you posted above. The min/max button was shown incorrect and the lookup image in a lookup wasen't displayed. So long Sascha
          Hide
          Jacques Le Roux added a comment -

          Hi Sascha,

          Yes I agree, I figured out yesterday that the min/max button was not needed at all, though I was not quite sure at this time. Unfortunately I was not able to apply your patch because it's oudated (Tomahawk theme only), could you please update and create a new patch?

          TIA

          Show
          Jacques Le Roux added a comment - Hi Sascha, Yes I agree, I figured out yesterday that the min/max button was not needed at all, though I was not quite sure at this time. Unfortunately I was not able to apply your patch because it's oudated (Tomahawk theme only), could you please update and create a new patch? TIA
          Hide
          Sascha Rodekamp added a comment -

          OK Jacques here we go LayoutFix.patch update against the latest trunk

          So long

          Sascha

          Show
          Sascha Rodekamp added a comment - OK Jacques here we go LayoutFix.patch update against the latest trunk So long Sascha
          Hide
          Jacques Le Roux added a comment -

          Thanks Sascha,

          Your patch is in trunk at r936008. However the main reason(min/max button) is still not fixed in the case of lookups rendered from FTL files. I guess it's because it's not the same rendering mechanism.

          Show
          Jacques Le Roux added a comment - Thanks Sascha, Your patch is in trunk at r936008. However the main reason(min/max button) is still not fixed in the case of lookups rendered from FTL files. I guess it's because it's not the same rendering mechanism.
          Hide
          Sascha Rodekamp added a comment -

          Hi Jacques, jep i had the same issue in my project, when calling the lookups directly. I will have a look this night and try to fix it

          So long Sascha

          Show
          Sascha Rodekamp added a comment - Hi Jacques, jep i had the same issue in my project, when calling the lookups directly. I will have a look this night and try to fix it So long Sascha
          Hide
          Sascha Rodekamp added a comment -

          Hi Jacques,

          this little patch should fix the layout bugs when the lookup is called directly from a n ftl file.
          (It fixes the problems in Tomahawk with the min/max button). If you find more layout bugs relating this issue, cann you make a screenshot for me ??

          Have a nice day

          Sascha

          Show
          Sascha Rodekamp added a comment - Hi Jacques, this little patch should fix the layout bugs when the lookup is called directly from a n ftl file. (It fixes the problems in Tomahawk with the min/max button). If you find more layout bugs relating this issue, cann you make a screenshot for me ?? Have a nice day Sascha
          Hide
          Jacques Le Roux added a comment -

          Hi Sascha,

          Which the layout bugs (when the lookup is called directly from FTL) should this patch resolves? Because as you can see in the "Min-max issue from FTL" there is still the error after having applied it. Note that it does not depend of the theme. BTW this should be better addressed in the issue I specially created for this problem: OFBIZ-3692.

          Also it supposes that you have applied the patch above (OFBIZ-3442 replace popup lookups by layered lookups.patch) on a clean trunk checkout

          Thanks

          Show
          Jacques Le Roux added a comment - Hi Sascha, Which the layout bugs (when the lookup is called directly from FTL) should this patch resolves? Because as you can see in the "Min-max issue from FTL" there is still the error after having applied it. Note that it does not depend of the theme. BTW this should be better addressed in the issue I specially created for this problem: OFBIZ-3692 . Also it supposes that you have applied the patch above ( OFBIZ-3442 replace popup lookups by layered lookups.patch) on a clean trunk checkout Thanks
          Hide
          Sascha Rodekamp added a comment -

          Ah ok, i tohught you mean the missplacing of the min/max button in Tomahawk theme, and the text 'Advanced Search' had a line break because of a to small div. This is what the patch fixes Just two simple layout issues

          Show
          Sascha Rodekamp added a comment - Ah ok, i tohught you mean the missplacing of the min/max button in Tomahawk theme, and the text 'Advanced Search' had a line break because of a to small div. This is what the patch fixes Just two simple layout issues
          Hide
          Jacques Le Roux added a comment -

          Thanks for clarification Sascha,

          Your patch is in trunk at r937191

          Show
          Jacques Le Roux added a comment - Thanks for clarification Sascha, Your patch is in trunk at r937191
          Hide
          Jacques Le Roux added a comment -

          Completed: At revision: 937852

          Show
          Jacques Le Roux added a comment - Completed: At revision: 937852

            People

            • Assignee:
              Jacques Le Roux
              Reporter:
              Jacques Le Roux
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development