myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sylvain Vieujot <svieu...@apache.org>
Subject Re: [Myfaces-develop] New feature suggestion : Edit mode
Date Mon, 09 May 2005 14:33:29 GMT
Hello Thomas,

Thanks for your reply.

I don't think this would work, and here are the reasons for that :

1) First, I find it more complex to add a new set of renderers than just
to add another attribute.

2) This doesn't apply only to the text/textarea renderers.
It would need to be implemented too for check boxes, radio buttons,
inputHtml, inputDate and any further/new components that would implement
this behavior.

3) Most important, the ultimate benefit of this is to allow the creation
of a new tag that sets this attribute for all it's children.
This allows you to have a whole editable/un-editable sections, and
really reduce the complexity & verbosity of the page.
To do this, the components (and not the renderers) have to implement an
interface (similar to what's done with the  UserRoleAware interface).

So, I don't see any benefit of doing a new sets of components and
overloading the renderers add this behaviour.
Further more, if we go along that path and want to be consistent, we
should do the same with the enabledOnUserRole and visibleOnUserRole
attributes, which would leave us with x: components what would be rather
similar to h: components. Do we really want h:, x: and xx: components ?

Just a change from what I thought before, I don't think we need the
matching style & styleClass attributes, as we can do without in CSS.

So, this leaves us with just :
- one interface and one attribute to implement,
- a new component to set this attributes on it's children.

Do you agree with that, or do you think I'm wrong somewhere ?

Thank you,

Sylvain.

On Mon, 2005-05-09 at 15:50 +0200, Thomas Spiegl wrote:

> Sylvain,
> We just need to overwrite the encodeEnd methods of HtmlTextRenderer,
> HtmlMenuRenderer and HtmlTextareaRenderer. An output will be rendered
> if readonly is set true, otherwise rendering will be delegated by
> calling super.encodeEnd
> WDYT
> 
> regards, thomas
> 
> On 5/6/05, Sylvain Vieujot <svieujot@apache.org> wrote:
> >  What about the displayOnly or displayValueOnly attribute ?
> > 
> >  
> >  On Thu, 2005-05-05 at 22:40 -0400, Sylvain Vieujot wrote:
> >  
> >  Hello Thomas,
> >  
> >  I agree that Myfaces' extended components shouldn't get overloaded with new
> > functionality & attributes.
> >  Now, if I understand well your suggestion, and if I try to implement it, I
> > think this leads to a way more complex code within MyFaces'.
> >  Dealing with this new attribute in the current code is quite straight
> > forward.
> >  Dealing with a bunch of new renderers isn't as simple.
> >  
> >  Also, to me having an attribute (readonly) that has 2 different behaviours
> > would lead to more misunderstanding than a tag that you can just forget if
> > you don't have use to it.
> >  
> >  I think the misunderstanding (editable confused with the readonly) has in
> > fact more to do with the attribute's name.
> >  So, maybe we should find something more self explanatory than editable.
> >  I don't have a better one to suggest right now, but I'll try hard to find
> > one.
> >  
> >  Sylvain.
> >  
> >  On Thu, 2005-05-05 at 21:47 +0200, Thomas Spiegl wrote: 
> >  I'd suggest to use the readonly attribute in association with a new 
> > renderer type and new jsp-tags. The advantage would be, that the same 
> > attributes could be used as for the existing components. Just the 
> > rendering behaviour concerning the readonly attribute would be 
> > different. Myfaces' extended components shouldn't get overloaded with 
> > new functionality & attributes, above all this could lead to 
> > misunderstandings (as Rolf mentioned).
> > I think new tags would bring out the differences more clearly.
> > Anyway the InputText's readonly attribute should be set to false, when 
> > rendered as OutputText. Otherwise the decode method would not work correct!
> > 
> > naming proposal:
> > renderer-type: org.apache.myfaces.Text2
> > renderer-class: HtmlTextRenderer2 (extending 
> > org.apache.myfaces.renderkit.html.ext.HtmlTextRenderer)
> > tag-names: inputText2, ..
> > 
> > This would bring clear separtion and would not mean too much effort.
> > 
> > regards, Thomas
> > 
> > >inputText should definitely support @editable. If I understand you guys
> > >correct, you think editable="false" means, the component is rendered as
> > >an outputText. IMHO it should be rendered as a readonly input field.
> > >
> > ><input ... readonly="true" .../>
> > >
> > >Otherwise, two different concepts/semantics are mixed, which is bad
> > >imho.
> > >
> > >Makes sense?
> > >
> > > 
> > >
> > 
> > >yes, that is exactly my problem as well. look at the code below - I
> > >generally have 20 fields like that per form, unmanageable at all:
> > >
> > > <h:panelGroup>
> > > <h:inputText id="editableRequestLimit"
> > >value="#{limitDetail.limitView.requestLimit}"
> > styleClass="text"
> > > converter="#{converterBean.amountConvt}"
> > >rendered="#{limitDetail.limitView.changeable}"/>
> > > <h:outputText
> > >value="#{limitDetail.limitView.requestLimit}"
> > styleClass="text"
> > > converter="#{converterBean.amountConvt}"
> > >rendered="#{!limitDetail.limitView.changeable}"/>
> > > </h:panelGroup>
> > >
> > >In comparison, that looks rather nice:
> > >
> > > <h:inputText id="editableRequestLimit"
> > >value="#{limitDetail.limitView.requestLimit}"
> > styleClass="text"
> > > converter="#{converterBean.amountConvt}" 
> > >editable="#{limitDetail.limitView.changeable}"/>
> > >
> > >what we do when requesting the user to duplicate the definition is in
> > >fact create redundancy - I believe that this is unnecessary. And
> > >really, that is very much the same as the existing editableOnUserRole
> > >and the visibleOnUserRole attributes - you change the rendering
> > >dependent on an attribute.
> > >
> > >I like Sean's idea of dynamically deciding upon a renderer though, but
> > >I am not sure if that is possible with the framework at all...
> > >
> > >regards,
> > >
> > >Martin
> > >
> > >
> > >On 5/5/05, Sylvain Vieujot <svieujot@apache.org> wrote:
> > > 
> > >
> > >> I came up with this idea, as I come from a Notes/Domino background, where
> > >>you can have a document or document sections in edit mode or in read mode.
> > >> 
> > >> Sure, you can do this with multiple fields (at least always 2), but this
> > >>quickly turns into a maintenance nightmare, with pages very hard to read.
> > >> 
> > >> When you have pages with 50+ components that can be either in edit mode
> > or
> > >>read mode, this becomes quite complex and generate bugs.
> > >> It's harder to test to, as you have to test all modes to be sure your
> > >>edit/output components are consistent.
> > >> So, the easiest place to solve this is into the components, as it makes
> > the
> > >>application easier design and to maintain.
> > >> 
> > >> This is the same logic as the one that leads to the creation of the
> > >>enabledOnUserRole like attributes.
> > >> 
> > >> A small example to show how it can greatly simplify a real-life example
:
> > >> 
> > >> In a client form, I have a section with the accounting informations that
> > >>the account manager can view, but not edit, and that only the accounting
> > >>department can edit, it the client has some status.
> > >> So, the formula to know it a field in this section is editable is quite
> > >>complex.
> > >> 
> > >> If I use the components without this extension, I have roughly 48
> > >>components, and 32 of them have a complex rendered formula.
> > >> This is both inefficient, and hard to maintain.
> > >> Now, if I use the editable attribute, I only have 16 fields, which is a
> > >>great improvement maintenance wise.
> > >> If now I put all those fields in an x:panelGroup with the editable
> > >>attribute/formula, I still have those 16 fields (17 with the x:panelGroup
> > >>one), but the editable formula is writen just once.
> > >> In addition to this, this is just a section of the page. In my
> > application,
> > >>just for the main client form, I have 6 sections. 4 of which having custom
> > >>editable settings.
> > >> So, for the all form, the difference is about 128 additional components,
> > >>and 124 additional render formulas.
> > >> This is definitely a real improvement, and the difference between a code
> > >>cluttered page, and a page that can be readable & maintainable.
> > >> 
> > >> Even if the concept can seem useless and just meant to add more
> > attributes
> > >>and/or gadgets to the x: components, in large applications, it has some
> > real
> > >>value.
> > >> 
> > >> Sylvain.
> > >>
> > >> 
> > >> On Wed, 2005-05-04 at 15:32 -0400, Sean Schofield wrote: 
> > >> Martin has a good point about the panel group. That does make it a
> > >>bit more cumbersome (4 lines instead of 1.) Although you could argue
> > >>that the approach I suggested is still more transparent.
> > >>
> > >>This is the approach I took with tree2. To use standard JSF
> > >>conventions to solve a problem whenever possible b/c this is the
> > >>common framework that all JSF programmers will be familiar with. So
> > >>instead of rendering html links in the tree, just let the user specify
> > >>whether they want one or not using h:commandLink and a facet. So that
> > >>is the argument for simplicity/flexability and the expense of a few
> > >>extra lines.
> > >>
> > >>Rolf's idea concerning a "dynamic renderer" sounds interesting,
> > >>however. Perhaps we could explore that a little more. JSF of course
> > >>allows you to specify which renderer to use for which component but
> > >>that is done statically via the faces-config.xml, etc. It might be
> > >>interesting to have several different renderers for a piece of data
> > >>that you could swap based on the results of a value binding
> > >>expression.
> > >>
> > >>So you could use the existing text component, create a custom tag and
> > >>then create 1...n custom renderers. This is probably what you would
> > >>do anyways even if you didn't have the renderer be dynamic right? The
> > >>added step could be that you could determine how to render
> > >>dynamically.
> > >>
> > >>Another use case could be a list of values. If the field should not
> > >>be editable you could make the current selection displayed as plain
> > >>text (instead of disabling the combo box.)
> > >>
> > >>My 2 cents, (US$ so less than 2 Euro Cents at the moment)
> > >>
> > >>sean
> > >>
> > >>On 5/4/05, Martin Marinschek <martin.marinschek@gmail.com> wrote:
> > >> 
> > >>
> > >>>of course - it's just the rendering that is different.
> > >>>
> > >>>regards,
> > >>>
> > >>>Martin
> > >>>
> > >>>On 5/4/05, Rolf Kulemann <roku@apache.org> wrote:
> > >>> 
> > >>>
> > >>>>On Wed, 2005-05-04 at 18:44, Martin Marinschek wrote:
> > >>>> 
> > >>>>
> > >>>>>Well, MyFaces tries to remain in the Spec as much as possible
in any
> > >>>>>of the core classes and the core components - extended components
have
> > >>>>>far left the functionality described in the spec ;)
> > >>>>>
> > >>>>>so these attributes would of course only be implemented in the
x:...
> > >>>>> 
> > >>>>>
> > >>components
> > >> 
> > >>
> > >>>>Yes, but I still think, that inputText and outputText semantics
should
> > >>>>not be mixed, whether in the x:* components or somewhere else. A
> > >>>>inputText is and inputText and not a outputText, although a inputText
> > >>>>can be non-editable, but is stays an inputText.
> > >>>>
> > >>>>Sorry for my blue-eyedness :)
> > >>>>
> > >>>>--
> > >>>>Rolf Kulemann
> > >>>>
> > >>>>
> > >>>> 
> > >>>>
> > >> 
> > >>
> > >
> > > 
> > >
> > 
> > 
> >

Mime
View raw message