OFBiz
  1. OFBiz
  2. OFBIZ-4090

Add Support For Compound Screen Widget XML Files

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Implemented
    • Affects Version/s: Trunk
    • Fix Version/s: Upcoming Branch
    • Component/s: framework
    • Labels:
      None
    • Sprint:
      Bug Crush Event - 21/2/2015

      Description

      Add the ability to include screen widgets, form widgets, menu widgets, and simple methods in a single XML file. This approach could be used in situations where the widgets share a logical grouping - so they can be kept in one place.

      1. OFBIZ-4090.patch
        215 kB
        Jacques Le Roux
      2. CompoundWidgetFiles.patch
        471 kB
        Adrian Crum
      3. CompoundWidgetFiles.patch
        8 kB
        Adrian Crum

        Issue Links

          Activity

          Hide
          Jacques Le Roux added a comment -

          Implemented at r1738958 in trunk. See OFBIZ-6990 for last details

          Show
          Jacques Le Roux added a comment - Implemented at r1738958 in trunk. See OFBIZ-6990 for last details
          Hide
          Jacques Le Roux added a comment -

          Actually I wanted to get further than what Adrian did but I did not. So you can find the change in his latest patch.

          Show
          Jacques Le Roux added a comment - Actually I wanted to get further than what Adrian did but I did not. So you can find the change in his latest patch.
          Hide
          Jacques Le Roux added a comment -

          Also mark the new Jira as "is depended upon by" the one here

          Show
          Jacques Le Roux added a comment - Also mark the new Jira as "is depended upon by" the one here
          Hide
          Jacques Le Roux added a comment -

          Don't forget its subtask

          Show
          Jacques Le Roux added a comment - Don't forget its subtask
          Hide
          Jacques Le Roux added a comment -

          Simply close it as "Unresolved"

          Show
          Jacques Le Roux added a comment - Simply close it as "Unresolved"
          Hide
          james yong added a comment -

          Now I think the Example would be a better place to start with the Compound Widget Screen feature. But what should I do with OFBIZ-6978?

          Show
          james yong added a comment - Now I think the Example would be a better place to start with the Compound Widget Screen feature. But what should I do with OFBIZ-6978 ?
          Hide
          james yong added a comment -

          I found that it is ok to use the original xsd files. So the template can be something like

          <root xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
          
            <site-conf xmlns="http://ofbiz.apache.org/Site-Conf"
                       xsi:schemaLocation="http://ofbiz.apache.org/Site-Conf http://ofbiz.apache.org/dtds/site-conf.xsd">
          
                <!-- Insert controller entries here -->
          
            </site-conf>
          
            <simple-methods xmlns="http://ofbiz.apache.org/Simple-Method"
                            xsi:schemaLocation="http://ofbiz.apache.org/Simple-Method http://ofbiz.apache.org/dtds/simple-methods.xsd">
          
                <!-- Insert simple methods here -->
          
            </simple-methods>
          
            <menus xmlns="http://ofbiz.apache.org/Widget-Menu"
                   xsi:schemaLocation="http://ofbiz.apache.org/Widget-Menu http://ofbiz.apache.org/dtds/widget-menu.xsd">
          
                <!-- Insert menu widgets here -->
          
            </menus>
          
            <forms xmlns="http://ofbiz.apache.org/Widget-Form"
                   xsi:schemaLocation="http://ofbiz.apache.org/Widget-Form http://ofbiz.apache.org/dtds/widget-form.xsd">
          
                <!-- Insert form widgets here -->
          
            </forms>
          
            <screens xmlns="http://ofbiz.apache.org/Widget-Screen"
                   xsi:schemaLocation="http://ofbiz.apache.org/Widget-Screen http://ofbiz.apache.org/dtds/widget-screen.xsd">
          
                <!-- Insert screen widgets here -->
          
            </screens>
          
          </root>
          
          
          
          Show
          james yong added a comment - I found that it is ok to use the original xsd files. So the template can be something like <root xmlns:xsi = "http://www.w3.org/2001/XMLSchema-instance" > <site-conf xmlns= "http://ofbiz.apache.org/Site-Conf" xsi:schemaLocation= "http://ofbiz.apache.org/Site-Conf http://ofbiz.apache.org/dtds/site-conf.xsd" > <!-- Insert controller entries here --> </site-conf> <simple-methods xmlns= "http://ofbiz.apache.org/Simple-Method" xsi:schemaLocation= "http://ofbiz.apache.org/Simple-Method http://ofbiz.apache.org/dtds/simple-methods.xsd" > <!-- Insert simple methods here --> </simple-methods> <menus xmlns= "http://ofbiz.apache.org/Widget-Menu" xsi:schemaLocation= "http://ofbiz.apache.org/Widget-Menu http://ofbiz.apache.org/dtds/widget-menu.xsd" > <!-- Insert menu widgets here --> </menus> <forms xmlns= "http://ofbiz.apache.org/Widget-Form" xsi:schemaLocation= "http://ofbiz.apache.org/Widget-Form http://ofbiz.apache.org/dtds/widget-form.xsd" > <!-- Insert form widgets here --> </forms> <screens xmlns= "http://ofbiz.apache.org/Widget-Screen" xsi:schemaLocation= "http://ofbiz.apache.org/Widget-Screen http://ofbiz.apache.org/dtds/widget-screen.xsd" > <!-- Insert screen widgets here --> </screens> </root>
          Hide
          james yong added a comment -

          The change for ConfigXMLReader.java is missing in the current patch file.

          Show
          james yong added a comment - The change for ConfigXMLReader.java is missing in the current patch file.
          Hide
          Jacques Le Roux added a comment -

          From what I have seen I think I will keep this issue open for now and we will close it once the work on OFBIZ-6978 will be done, thanks!

          Show
          Jacques Le Roux added a comment - From what I have seen I think I will keep this issue open for now and we will close it once the work on OFBIZ-6978 will be done, thanks!
          Hide
          james yong added a comment -

          Thanks for the pointers.
          I think the consensus is to work things out by moving forward with the Quote Screen refactoring.
          So a new issue at OFBIZ-6978.

          Show
          james yong added a comment - Thanks for the pointers. I think the consensus is to work things out by moving forward with the Quote Screen refactoring. So a new issue at OFBIZ-6978 .
          Hide
          Jacques Le Roux added a comment -

          If you are sure about what you do, why not indeed.

          But remember that the current simple-methods.xsd must miss the elements which were initially provided by the simple-methods-v2.xsd in the CompoundWidgetFiles.patch from 03/Jan/11 02:27. I try to find them but gave up. I think It should not be that hard to find and put them in our current simple-methods.xsd, but I did not :/.

          Regarding splitting patches it's easier, for me at least, to have only 1 patch. Or if it's really hard to review several smaller patches in only one Jira issue. This at least if it makes sense from a functional perspective. Else better to create several Jira issues (seems not the case here).

          It's fun to notice that by splitting issues and patches we would work contrary to what the compound screen feature try to avoid (opening, searcing in more files, etc.)

          And last but not least, Adrian said that, even with his complete changes, the Eclipse auto-completion feature was not working properly. So It's interesting but I feel there are more work in your plate that maybe you envision

          Thanks to try anyway!

          Show
          Jacques Le Roux added a comment - If you are sure about what you do, why not indeed. But remember that the current simple-methods.xsd must miss the elements which were initially provided by the simple-methods-v2.xsd in the CompoundWidgetFiles.patch from 03/Jan/11 02:27. I try to find them but gave up. I think It should not be that hard to find and put them in our current simple-methods.xsd, but I did not :/. Regarding splitting patches it's easier, for me at least, to have only 1 patch. Or if it's really hard to review several smaller patches in only one Jira issue. This at least if it makes sense from a functional perspective. Else better to create several Jira issues (seems not the case here). It's fun to notice that by splitting issues and patches we would work contrary to what the compound screen feature try to avoid (opening, searcing in more files, etc.) And last but not least, Adrian said that, even with his complete changes, the Eclipse auto-completion feature was not working properly. So It's interesting but I feel there are more work in your plate that maybe you envision Thanks to try anyway!
          Hide
          Pierre Smits added a comment -

          I suggest to create a new JIRA issue, and start from there.

          Show
          Pierre Smits added a comment - I suggest to create a new JIRA issue, and start from there.
          Hide
          james yong added a comment -

          Any problem if I refactor the Quote Screen to use the compound feature?
          Also thinking of creating sub tasks to make it easier to review the smaller patches.

          Show
          james yong added a comment - Any problem if I refactor the Quote Screen to use the compound feature? Also thinking of creating sub tasks to make it easier to review the smaller patches.
          Hide
          Jacques Le Roux added a comment -

          That would be great, but I'm not sure of the status currently. I think at least it misses some lines in (now in trunk) simple-methods.xsd and even if we get the same than Adrian did, he said he crossed issues with auto-completion in Eclipse. Anyway trying will enlight us for sure!

          Show
          Jacques Le Roux added a comment - That would be great, but I'm not sure of the status currently. I think at least it misses some lines in (now in trunk) simple-methods.xsd and even if we get the same than Adrian did, he said he crossed issues with auto-completion in Eclipse. Anyway trying will enlight us for sure!
          Hide
          james yong added a comment -

          This feature is useful. I am interested to refactor some screens like EditQuote with it.

          Show
          james yong added a comment - This feature is useful. I am interested to refactor some screens like EditQuote with it.
          Hide
          Jacques Le Roux added a comment - - edited

          I had to do some changes manually in ConfigXMLReader.java as well. I did not include the simple-methods-v2.xsd file, we have one already and they don't compare. So maybe things are missing in the new one

          Show
          Jacques Le Roux added a comment - - edited I had to do some changes manually in ConfigXMLReader.java as well. I did not include the simple-methods-v2.xsd file, we have one already and they don't compare. So maybe things are missing in the new one
          Hide
          Jacques Le Roux added a comment -

          Here a new and last patch, nothing tested for now. I had to do some changes manually in SimpleMethod.java but they sounds OK

          Show
          Jacques Le Roux added a comment - Here a new and last patch, nothing tested for now. I had to do some changes manually in SimpleMethod.java but they sounds OK
          Hide
          Jacques Le Roux added a comment -

          Mmm, it seems more work should be done to create an updated patch. I'll have a look...

          Show
          Jacques Le Roux added a comment - Mmm, it seems more work should be done to create an updated patch. I'll have a look...
          Hide
          Jacques Le Roux added a comment -

          I think this is a very interesting feature and we should try to commit it before the patch becomes unusable. So I have created a new patch w/o the simple-methods-v2.xsd because it now already exists

          BTW I think we should remove

          <system systemId="http://ofbiz.apache.org/dtds/simple-methods.xsd" uri="simple-methods.xsd" />
          

          from minilang-catalog.xml. It's not used anymore.

          Show
          Jacques Le Roux added a comment - I think this is a very interesting feature and we should try to commit it before the patch becomes unusable. So I have created a new patch w/o the simple-methods-v2.xsd because it now already exists BTW I think we should remove <system systemId= "http: //ofbiz.apache.org/dtds/simple-methods.xsd" uri= "simple-methods.xsd" /> from minilang-catalog.xml. It's not used anymore.
          Hide
          Adrian Crum added a comment -

          The implementation basically worked, but the Eclipse editor wasn't working well with the changes.

          The patch is very outdated. It will need to be updated.

          Show
          Adrian Crum added a comment - The implementation basically worked, but the Eclipse editor wasn't working well with the changes. The patch is very outdated. It will need to be updated.
          Hide
          Jacques Le Roux added a comment -

          HI Adrian, what's the status here?

          Show
          Jacques Le Roux added a comment - HI Adrian, what's the status here?
          Hide
          Adrian Crum added a comment -

          Here is the root schema I tried:

          <xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
                     elementFormDefault="qualified">
              <xs:element name="root">
                  <xs:complexType>
                      <xs:all minOccurs="0" maxOccurs="1">
                          <xs:element name="site-conf"/>
                          <xs:element name="simple-methods"/>
                          <xs:element name="forms"/>
                          <xs:element name="menus"/>
                          <xs:element name="trees"/>
                          <xs:element name="screens"/>
                      </xs:all>
                  </xs:complexType>
              </xs:element>
          </xs:schema>
          

          In Eclipse, you can right-click on an XML element and a popup menu appears with valid selections for that element - based on the schema. I can't provide a screen shot because now the popup menu doesn't show any suggestions from the schema. I'm almost certain it's a bug in Eclipse's XML editor.

          Show
          Adrian Crum added a comment - Here is the root schema I tried: <xs:schema xmlns:xs= "http: //www.w3.org/2001/XMLSchema" elementFormDefault= "qualified" > <xs:element name= "root" > <xs:complexType> <xs:all minOccurs= "0" maxOccurs= "1" > <xs:element name= "site-conf" /> <xs:element name= "simple-methods" /> <xs:element name= "forms" /> <xs:element name= "menus" /> <xs:element name= "trees" /> <xs:element name= "screens" /> </xs:all> </xs:complexType> </xs:element> </xs:schema> In Eclipse, you can right-click on an XML element and a popup menu appears with valid selections for that element - based on the schema. I can't provide a screen shot because now the popup menu doesn't show any suggestions from the schema. I'm almost certain it's a bug in Eclipse's XML editor.
          Hide
          Paul Foxworthy added a comment -

          Is it easy for you to post a screenshot showing the problem? I don't use Eclipse, so I can't see what you're seeing.

          Show
          Paul Foxworthy added a comment - Is it easy for you to post a screenshot showing the problem? I don't use Eclipse, so I can't see what you're seeing.
          Hide
          Adrian Crum added a comment -

          The root element could be called anything. "widgets" sounds good to me.

          I created a schema for the root element, but I couldn't get it to work. The file would parse fine, but in Eclipse all of the elements were shifted down one level - meaning auto-complete would insert values from child elements, not the selected element. Maybe it's just a problem with Eclipse - I don't know.

          If anyone can improve upon the patch, they are welcome to do so.

          Show
          Adrian Crum added a comment - The root element could be called anything. "widgets" sounds good to me. I created a schema for the root element, but I couldn't get it to work. The file would parse fine, but in Eclipse all of the elements were shifted down one level - meaning auto-complete would insert values from child elements, not the selected element. Maybe it's just a problem with Eclipse - I don't know. If anyone can improve upon the patch, they are welcome to do so.
          Hide
          Paul Foxworthy added a comment -

          I would prefer a more descriptive name than "root" for the root element. What about "widgets" or "ofbiz-widgets"?

          Shouldn't there be a schema for the root element, whatever name it has, specifying the valid name for the root element and valid subelements within it? It would be a very simple schema compared to the ones for screen widgets etc, but should be there for completeness, I think. Any reason why not?

          Show
          Paul Foxworthy added a comment - I would prefer a more descriptive name than "root" for the root element. What about "widgets" or "ofbiz-widgets"? Shouldn't there be a schema for the root element, whatever name it has, specifying the valid name for the root element and valid subelements within it? It would be a very simple schema compared to the ones for screen widgets etc, but should be there for completeness, I think. Any reason why not?
          Hide
          Adrian Crum added a comment -

          Updated patch file. XML parsing errors have been fixed with new schemas. Modifying the existing schemas didn't work because that caused parsing errors on existing screens (~1MB log entries per request).

          This patch also adds the ability to include a controller <site-conf> element in the compound widget file - an idea suggested by David.

          It all works and it is ready to commit if the community agrees it is a worthwhile improvement.

          Show
          Adrian Crum added a comment - Updated patch file. XML parsing errors have been fixed with new schemas. Modifying the existing schemas didn't work because that caused parsing errors on existing screens (~1MB log entries per request). This patch also adds the ability to include a controller <site-conf> element in the compound widget file - an idea suggested by David. It all works and it is ready to commit if the community agrees it is a worthwhile improvement.
          Hide
          Adrian Crum added a comment - - edited

          The attached patch demonstrates the concept.

          There is an issue preventing it from being committed: The XML validator reports errors because the schemas don't contain a target namespace. If I add a target namespace to the schemas, the validator reports errors because it was expecting the target namespace to be empty. I don't know how to solve that problem and I'm hoping an XML expert can figure it out.

          Show
          Adrian Crum added a comment - - edited The attached patch demonstrates the concept. There is an issue preventing it from being committed: The XML validator reports errors because the schemas don't contain a target namespace. If I add a target namespace to the schemas, the validator reports errors because it was expecting the target namespace to be empty. I don't know how to solve that problem and I'm hoping an XML expert can figure it out.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development

                  Agile