Bugzilla – Full Text Bug Listing
| Summary: | Allow creation of custom form editor pages | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Rob Cernich <rcernich> | ||||||
| Component: | Sapphire | Assignee: | Konstantin Komissarchik <konstantin> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | gregory.amerson, konstantin | ||||||
| Version: | unspecified | Keywords: | plan | ||||||
| Target Milestone: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Rob Cernich
Created attachment 204839 [details]
Patch v1
A proposed implementation for the feature. Adds the following UI model types:
ISapphireFormEditorPageDef - represents a form editor page comprised of sections.
ISapphireFormEditorSectionDef - represents a section on a form editor page.
Form editor pages are comprised solely of sections.
I have built an editor using these components and this implementation appears to work fine. I have not tested adding actions to the editor (i.e. actions in the header bar), so I'm not sure if further work is required to support that. Same goes for actions in the section headers.
It would be great if this could get into the 0.3.1 release. Looks very promising. I haven't tried running this version. Here are comments for v1 based on code review of the patch: 1. It would be better if the new subclass of SapphireEditorFormPage went into its own class file. Maybe call it StandardFormEditorPage. We use "standard" prefix throughout the framework to indicate common/default scenario, so this should be a good fit. 2. Sapphire labels (in sdef or in various annotations) should not be capitalized. See http://www.eclipse.org/sapphire/documentation/latest/focus/localization/index.html for more info. 3. The default number of columns should probably be something other than 1. Maybe two would be more common? 4. It looks like something in your environment is re-formatting classes that you touch. See SapphireEditorFormPage outside the actual changes. I prefer to see clean patches without formatting changes that make it harder to see what was actually changed. 5. We also need a sample and an entry in 0.4 enhancements doc. Perhaps add a new page to the gallery sample? Regarding headers, etc... I would like to keep these in consistent format across the project. Here is an example to copy an modify: /****************************************************************************** * Copyright (c) 2011 Oracle and Others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * * Contributors: * Konstantin Komissarchik - initial implementation * Ling Hao - [329114] rewrite context help binding feature * Greg Amerson - [343972] Support image in editor page header ******************************************************************************/ Make sure to add the line like above under contributors for any file you modify. The @author tags for files you create or make significant contributions to should include e-mail address. Like so: @author <a href="mailto:konstantin.komissarchik@oracle.com">Konstantin Komissarchik</a> (In reply to comment #3) > 1. It would be better if the new subclass of SapphireEditorFormPage went into > its own class file. Maybe call it StandardFormEditorPage. We use "standard" > prefix throughout the framework to indicate common/default scenario, so this > should be a good fit. > Sounds good. The name I was thinking of matched the containing class, so I just stuck it in there as a static. I'll push it out into a separate class with a "Standard" prefix. > 2. Sapphire labels (in sdef or in various annotations) should not be > capitalized. See > http://www.eclipse.org/sapphire/documentation/latest/focus/localization/index.html > for more info. > Will do. > 3. The default number of columns should probably be something other than 1. > Maybe two would be more common? > Sounds good to me. I don't particularly have a preference, but am using two in my editor pages. > 4. It looks like something in your environment is re-formatting classes that > you touch. See SapphireEditorFormPage outside the actual changes. I prefer to > see clean patches without formatting changes that make it harder to see what > was actually changed. > Could you provide a link to an Eclipse code style settings? I could not find one in any of the Sapphire pages and the source does not appear to be formatted using any of the built-in Eclipse code style settings. > 5. We also need a sample and an entry in 0.4 enhancements doc. Perhaps add a > new page to the gallery sample? > No problem. > Regarding headers, etc... I would like to keep these in consistent format > across the project. Here is an example to copy an modify: > > /****************************************************************************** > * Copyright (c) 2011 Oracle and Others > * All rights reserved. This program and the accompanying materials > * are made available under the terms of the Eclipse Public License v1.0 > * which accompanies this distribution, and is available at > * http://www.eclipse.org/legal/epl-v10.html > * > * Contributors: > * Konstantin Komissarchik - initial implementation > * Ling Hao - [329114] rewrite context help binding feature > * Greg Amerson - [343972] Support image in editor page header > > ******************************************************************************/ > > Make sure to add the line like above under contributors for any file you > modify. > > The @author tags for files you create or make significant contributions to > should include e-mail address. Like so: > > @author <a href="mailto:konstantin.komissarchik@oracle.com">Konstantin > Komissarchik</a> > Not a problem. It may be a week or two before I get back to this, but hopefully I can have something for your 0.4 release. Best, Rob > Could you provide a link to an Eclipse code style settings? There isn't a single code style adopted by every Eclipse project. At Sapphire, committers and contributors are free to utilize whatever code style they are most comfortable with. The only rule is that when modifying an existing class, to not reformat portions of the class not being worked on. It is fine to use a different style in the section that is being worked on. Some developers like to turn on "auto-format on save" option or have a habit of invoking class format actions at times. We ask that developers refrain from doing so when contributing to Sapphire. Does that clarify the situation? > It may be a week or two before I get back to this, but hopefully I can have > something for your 0.4 release. That would be great. The 0.4 release will ship sometime in December. Do you still intend to complete this in time for 0.4 contribution deadline (11/17 to get the patch reviewed and merged) or should we re-target this to 0.5 release at this point? (In reply to comment #6) > Do you still intend to complete this in time for 0.4 contribution deadline > (11/17 to get the patch reviewed and merged) or should we re-target this to 0.5 > release at this point? I've got everything cleaned up and and am currently working on a sample. Should be good for 0.4. Created attachment 206991 [details]
Patch v2
Updated patch.
Cleaned up header comments and contributors list as requested.
Added a form editor page to the architecture sample.
Fixed some layout problems found while creating the sample. (I'll create a separate defect for the remaining layout problems.)
*** Bug 363755 has been marked as a duplicate of this bug. *** I have committed Patch v2 with some changes as it is a solid start even though quite a bit more work is still necessary. The most significant change that I made to the patch is resolution of LinkedToSection hack. I resolved it by making the search algorithm not bail out at section, but to search further for the root. In playing around with this feature, I am not thrilled with the layout methodology of columns/row-span/col-span. A better approach would be to use nested split forms. The split form is a new feature in 0.4 release. It has orientation (vertical/horizontal) and x number of compartments. The developer assigns weight to each compartments that determines the initial size. There is a splitter between each compartment. You can nest split forms. When nesting, you would typically change orientation relative to the parent. I am thinking that we can directly re-use the split form feature to in implementing the generic form editor page feature. Let's say the generic form editor page has form content (as opposed to sections in its definition). The developer could then add a split form and nest splits at whatever level. The only thing that's necessary is to ensure that section can appear as form part in any location that form content is accepted. Going this route would make the generic form editor page even more generic. For instance, if the developer felt like it, he would be able to place property editors directly on the page without sections. Let me know if that doesn't make any sense. Another aspect that I'd like to see improved is the sample. The approach of adding a page to the architecture sample doesn't do justice to this feature as it is going to be hard for someone to see why they would use this feature as opposed to the built-in master details page. A better sample would have more than two sections on a page, in several rows and without master-details relationship. It would also be best to do this in the gallery sample (or at least there) as the gallery sample is supposed to show every feature. (In reply to comment #10) > I have committed Patch v2 with some changes as it is a solid start even though > quite a bit more work is still necessary. The most significant change that I > made to the patch is resolution of LinkedToSection hack. I resolved it by > making the search algorithm not bail out at section, but to search further for > the root. > Sounds good. I was not sure if removing that barrier would break anything. That said, I'm guessing supporting "linked" sections would be a common use case as many form editors today exhibit this behavior. > In playing around with this feature, I am not thrilled with the layout > methodology of columns/row-span/col-span. A better approach would be to use > nested split forms. The split form is a new feature in 0.4 release. It has > orientation (vertical/horizontal) and x number of compartments. The developer > assigns weight to each compartments that determines the initial size. There is > a splitter between each compartment. You can nest split forms. When nesting, > you would typically change orientation relative to the parent. > Yeah, that wasn't there when I started this. I agree this sounds like a better way of laying out sections. > Another aspect that I'd like to see improved is the sample. The approach of > adding a page to the architecture sample doesn't do justice to this feature as > it is going to be hard for someone to see why they would use this feature as > opposed to the built-in master details page. A better sample would have more > than two sections on a page, in several rows and without master-details > relationship. It would also be best to do this in the gallery sample (or at > least there) as the gallery sample is supposed to show every feature. > Of course, but I didn't have a lot of time to put this together. Part of the reason behind such a simplistic sample is that it is difficult to provide controls for non-existent model elements, especially if the element is nested more than one level beyond the root (e.g. "Dependencies" page on the Maven POM editor provides the ability to edit the contents of the "dependencies" node, as well as the "dependencyManagement/dependencies" node). (My limited understanding is that this must be accomplished using "with," which forces the injection of a checkbox which must be checked prior to adding a child, or perhaps it has to do with the way I've structured my models.) Thanks for working through the patch. Best, Rob If you haven't started already, I think I will take a stab this morning at re-working this feature to use split form functionality like I described in the previous comment. Let me know either way. Regarding the sample and the element property issues. You are indeed correct that you have to use the with construct. What seems to be tripping you up is the distinction between ElementProperty and ImpliedElementProperty. You use the with construct for both of them, but for the implied kind, there is no checkbox for explicitly creating the element. To see the difference in action, take a look at the Address and Assistant properties in the contacts sample. I completed the conversion of this feature to use the split form mechanism as described in Comment #10. The sections now support expand vertically setting. For the sample, I tried to come up with something that would really showcase why you would use this feature. I ended up creating a completely new sample to do that... The purchase order sample (org.eclipse.sapphire.samples.po). To try out the sample, create a file with .po extension. Take this feature for a spin to make sure it works for your requirements. We don't have a lot of time left to finalize it before the 0.4 release. I am not resolving this bug just yet as I want to improve the purchase order sample a bit and a section in the enhancements docs still needs to be written. I will go ahead and mark this as fixed now as that's necessary in order to get the patch to show up in the IP log, but I still need to write the doc content and flush out the purchase order sample a bit more. Hey Konstantin, Thanks for doing this. Sorry I haven't been able to give it a look; maybe over the holiday. Best, Rob The purchase order sample is now complete. I also added a section to the 0.4 enhancements guide. Hey Konstantin, I've updated my project to use the latest version of this feature and am experiencing some undesirable layout behavior. I'm guessing this is a result of using the new split form. While it is nice to be able to specify weightings to the columns, I don't think it transfers very well to rows. In my case, I have three rows: 1) horizontal split form 2) section 3) section In one of the sections, I'm using a page book and in the other I'm using a "with." When either of these sections is expanded (e.g. checking the checkbox on the with or selecting an item in the list associated with the page book), it adds a lot of white space to the other sections (as they're scaled using the weightings). This causes the widgets to really move around and makes for some ugly screens. I think a better option for rows would be to use the "scale" properties already common throughout the rest of the model. Other than that, it appears to function the same as the original patch. Rob Could you attach screen captures to illustrate the layout issues that you are seeing? I am having a hard time grasping what you are describing.
> I think a better option for rows would be to use the "scale" properties already
> common throughout the rest of the model.
You can use scale-vertically as well. Note that it isn't required that you use a separate split form block for every section. You can place multiple sections in a single block if you don't need to control space allocation between them.
See general page in purchase order sample for an example of multiple sections per block. See entries page for an example of the use of scale-vertically.
(In reply to comment #18) > You can use scale-vertically as well. Note that it isn't required that you use > a separate split form block for every section. You can place multiple sections > in a single block if you don't need to control space allocation between them. > Removing the vertical split-form (i.e. placing the three "rows" as direct content of the form) took care of the problem. Looks great. Thanks for the hard work. Rob closed. |