Hello Pavitra,
There's some comments inline, but I basically agree with all points.
Regards,
~ Simon
On 8/30/06, Pavitra Subramaniam <[EMAIL PROTECTED]> wrote:
Comments inline
- Pavitra
> -----Original Message-----
> From: Simon Lessard [mailto:[EMAIL PROTECTED]
> Sent: Tuesday, August 29, 2006 5:24 PM
> To: [email protected]
> Subject: Re: TrainRenderer using the new train selectors
>
> Added some inline comments
>
> On 8/29/06, Pavitra Subramaniam
> <[EMAIL PROTECTED]> wrote:
> >
> > Hello Simon, Jeanne,
> >
> > I have some comments on both your email exhanges. I have
> consolidated
> > all the items below as it was getting hard to read.
> >
> >
> > >>>>
> > - af|train::stop combinable with :selected, :disabled, :completed
> > (will probably become p_AFVisited) and :unvisited. You can append
> > :readOnly at the end of the result. So
> > af|train::stop:unvisited:readOnly is valid
> > >>>>
> >
> > - Do you think we need to support an
> "af|train::stop:visited:readOnly"
>
>
> I already support it. Icons are also resolved in availability
> order. That is if the stop is visited and read-only, the
> first icon in the lookup list will be
> af|train::stop:visited:read-only-icon, then
> af|train::stop:visited-icon and finally af|train::stop-icon.
>
> - I believe Jeanne wanted to have states that don't use camel case. So
> > readOnly should be read-only
>
>
> Easy change, I can do this.
>
> >>>>
> > - af|train::link
> > >>>>
> > - should this be ::stop-link or ::link good enough?
>
>
> Both work, however since this selector is often used in
> combination with the ::stop selector, I would keep it with
> ::link only. Here's a selector example I use for skinning links:
>
> af|train::stop:selected af|train::link
> {
> font-weight: bold;
> }
>
> I believe it's expressive enough. What is your opinion about it?
>
-1. I have to support a scenario where both the stop icons and the parent
train icons are clickable (and hence rendered as links, rather than <img>).
I also need to show different icons on hover, mousedown and selected. So in
such situations it may be clear to define something as a stop-link or
stop-icon-link etc. af|train::link is too generic. Do you agree?
Fair point, I'll implement that while waiting to find a solution to IE 6
nightmare.
>>>>
> > - af|train::join combinable with :disabled, :completed,
> :unvisited and
> > :outer (:outer is used to add joins outside the edge of the
> train. I
> > don't think many will use it, but it cost nothing and add more
> > customization
> > possibilities)
> > >>>>
> >
> > - :join-outer pseudo element is good enough. Is this used
> outside the
> > parent train icons or between the parent train icons and
> the overflow
> > (or stop)?
>
>
> The latter, parent train icons and the stops. Currently I
> render the parent train icons only when the extremity of the
> train is visible. I was not sure if it was making sense to
> show the parent train in the middle of the process. Speaking
> of which, I added 2 skin properties as well:
>
> -ora-render-parent-train boolean property to set if the
> parent train should be rendered -ora-visible-stop-count
> integer property determning the amount of stop to render
> before overflow. Invalid value result in a warning and using
> the default value of 6.
+1
>
> - do we still have the join-overflow? This comes between the
> overflow and
> > regular stops. It may be useful in cases where only these
> joins need
> > not be displayed.
>
>
> My current version use standard joins for them. It's easy to
> add but I wonder how often skinners will want to render a
> join between the overflows and the stops but not between the
> stop themselves. So I'm +0 on that
- It's the opposite case I am interested in. Skinners may 'not' want to
display a join between the overflow-prev & first stop AND the last stop &
overflow-next, but display joins between the regular stops. I have to
support this behavior. So it may be useful to have a join-overflow.
Not hard to implement so it will be done as well.
> >>>>
> > - af|train::overflow-start combinable with :disabled and :readOnly
> > - af|train::overflow-end combinable with :disabled, :unvisited and
> > :readOnly
> > >>>>
> >
> > - Do you think we should support :unvisited state on
> overflow-start?
> > This may be useful in cases where the train is not
> sequential? I have
> > a scenario where all stops in the train are enabled and
> user can jump
> > around any stop without a prescribed order.
>
>
> Easy to add and allow more customization, +1 for this.
>
> >>>>
> > I have the following valid suffixes: (I could not use ::content for
> > example since double :: is now prevented from Adam's change
> to prevent
> > some strange behavior it seem) -content (for example, the
> following is valid:
> > af|train::stop-content and
> > af|train::stop:selected-content. This selector refers to
> the link cell
> > af|(of
> > the train)
> > >>>>
> >
> > - I agree with Jeanne. Let's call it stop-content:selected. I don't
> > think we need stop-content:visited or
> stop-content:unvisited. It's an overkill.
>
>
> stop-content unvisited is automatically supported as the cell
> get the p_AFUnvisited class tagged on it.
>
> >>>>
> > -icon-block (as above but refers to the icon cell)
> > >>>>
> >
> > - Perhaps we should have a convention of using -block
> suffix for style
> > classes that go on <div> and -cell for styles that go on
> td. Is that
> > acceptable?
>
>
> I'm -1 on that as it expose markup used.
- Agree. -block should be -cell, although it still refers to a <td>, but I
guess it's the current convention
>
> >>>>
> > I use the same join rule as you, that is previous stop
> determine the
> > join state, except for the join after the selected stop. The only
> > exception are disabled stops, those have disabled joins on
> both sides.
> > >>>>
> >
> > - the join after the selected stop should ideally have the state of
> > the following stop. Isn't it?
>
>
> Yep, that's what I use.
>
> Do you think it will be easier to just show the left join of
> a disabled stop
> > as "disabled". So if an unvisited stop follows the disabled
> stop, the
> > join between them will show up as unvisited. At least this is how I
> > have implemented it. I use the same rule for displaying
> joins before overflows.
>
>
> I prefer to see both sides of a disabled stop being disabled
> as it should not be accessible from either way, using an
> unvisited join toward the end side could be seen as: "If you
> can get to the step after the disabled one, just hit previous
> and you'll be able to access the disable step".
+1
>
> >>>>
> > I also go rid of the separator, it's redundant as you can
> add padding
> > to -content selector. I also added two aliases
> .AFTrainContent:alias
> > and .AFTrainIconBlock. So you can add spacing between stop with
> > something like the following:
> > .AFTrainContent:alias
> > {
> > padding-left: 8px;
> > padding-right: 8px;
> > }
> > >>>>
> > When you say separator, do you mean the spacer that
> separates stops?
> > Where is the alias above included? Can you send me a HTML fragment?
>
>
> Yes I meant the spacer. AFTrainContent:alias is added to the
> content cell of stop, overflow-start and overflow-end, that
> way it never breaks the joins.
- Hmm. I tried this and it adds some white space around the icon. Maybe I
need to test this some more.
>
> Thanks
> > - Pavitra
> >
> >
> > > -----Original Message-----
> > > From: Simon Lessard [mailto:[EMAIL PROTECTED]
> > > Sent: Tuesday, August 29, 2006 11:43 AM
> > > To: [email protected]
> > > Subject: Re: TrainRenderer using the new train selectors
> > >
> > > Ok,
> > >
> > > Then if those selectors are ok for you as well Pavitra
> I'll make the
> > > changes to -outer and and -icon-cell tonight and upload
> the patch as
> > > well as a test skin I used.
> > >
> > >
> > > Regards,
> > >
> > > ~ Simon
> > >
> > > On 8/29/06, Jeanne Waldman <[EMAIL PROTECTED]> wrote:
> > > >
> > > > yep, it all makes sense.
> > > > I can see where you'd want to use a ::content. That would
> > > make sense.
> > > > We don't do this yet in any of our skinning keys, so I am fine
> > > > with the -content, since we do that all over the place. :)
> > > >
> > > > - Jeanne
> > > >
> > > > Simon Lessard wrote:
> > > >
> > > > > Oups, comments below
> > > > >
> > > > > On 8/29/06, Jeanne Waldman <[EMAIL PROTECTED]> wrote:
> > > > >
> > > > >>
> > > > >> one question below
> > > > >>
> > > > >> Simon Lessard wrote:
> > > > >>
> > > > >> > Hello Pavitra,
> > > > >> >
> > > > >> > I had to do about the same changes on my side. Here's
> > > my list of
> > > > >> selector
> > > > >> > and the rules I used:
> > > > >> >
> > > > >> > - af|train::stop combinable with :selected, :disabled,
> > > :completed
> > > > >> (will
> > > > >> > probably become p_AFVisited) and :unvisited. You
> can append
> > > > >> > :readOnly at the end of the result. So
> > > > >> > af|train::stop:unvisited:readOnly is valid
> > > > >> > - af|train::link
> > > > >> > - af|train::join combinable with :disabled, :completed,
> > > > >> > :unvisited
> > > > and
> > > > >> > :outer (:outer is used to add joins outside the
> edge of the
> > > > >> > train. I don't think many will use it, but it cost nothing
> > > > >> > and add more
> > > > customization
> > > > >> > possibilities)
> > > > >>
> > > > >> Is :outer a state? It sounds to me like it should be
> > > > >> af|train::join-outer
> > > > >
> > > > >
> > > > >
> > > > > yes, it could be ::join-outer, was made a state only
> to fit the
> > > > > other join selectors, but it does make more sense to use
> > > -outer for
> > > > > that one.
> > > > >
> > > > >
> > > > > Does that make more sense now?
> > > > >
> > > > >
> > > > > Regards,
> > > > >
> > > > > ~ Simon
> > > > >
> > > > >> - af|train::overflow-start combinable with :disabled and
> > > :readOnly
> > > > >> > - af|train::overflow-end combinable with :disabled,
> > > > >> > :unvisited and :readOnly
> > > > >> >
> > > > >> > I have the following valid suffixes: (I could not use
> > > ::content
> > > > >> > for example since double :: is now prevented from
> > > Adam's change
> > > > >> > to prevent some strange behavior it seem)
> > > > >>
> > > > >> I don't think two pseudo-elements make sense, does it? I
> > > > >> suppose you could have a piece of a piece. Adam prevented it
> > > because there
> > > > >> were bugs in it. I can't recall what the bugs were.
> > > > >
> > > > >
> > > > >
> > > > > I was seeing them more as sub-elements, like ::stop::content
> > > > > (content of the
> > > > > stop)
> > > > >
> > > > >> -content (for example, the following is valid:
> > > > >> af|train::stop-content
> > > > >> > and
> > > > >> > af|train::stop:selected-content. This selector refers
> > > to the link
> > > > cell
> > > > >> fo
> > > > >> > the train)
> > > > >>
> > > > >> What does a :selected-content 'state' mean?
> > > > >> How is it different than af|train::stop-content:selected?
> > > > >
> > > > >
> > > > >
> > > > > My bad there, af|train::stop-content:selected is actually
> > > what I use.
> > > > > Even
> > > > > if a better selector would have been
> > > > > af|train::stop:selected::content imho.
> > > > >
> > > > >> -icon-block (as above but refers to the icon cell)
> > > > >>
> > > > >> Could you say -icon-cell? We use 'cell' quite a bit in
> > > our skinning
> > > > >> selectors.
> > > > >
> > > > >
> > > > >
> > > > > Yes I could, I was using block only because it was in Pavitra
> > > > > document
> > > > at
> > > > > first.
> > > > >
> > > > >>
> > > > >> > The icons follow the same rule.
> > > > >> >
> > > > >> > On 8/28/06, Pavitra Subramaniam
> > > <[EMAIL PROTECTED]>
> > > > >> wrote:
> > > > >> >
> > > > >> >>
> > > > >> >> Hello Simon,
> > > > >> >>
> > > > >> >> I have also almost completed implementing the
> TrainRenderer
> > > > >> >> using
> > > > the
> > > > >> >> new
> > > > >> >> skin selectors. It's great to know you are done as
> > > well. If you
> > > > >> plan to
> > > > >> >> check in the train renderer code anytime soon, can we
> > > agree on
> > > > >> >> the common list of skin selectors, so that I can
> > > reuse them for
> > > > >> >> my work internally at Oracle? I had to make the following
> > > > >> >> changes and wanted to give you
> > > > an
> > > > >> >> update.
> > > > >> >>
> > > > >> >> 1. I had to introduce a new state called
> "read-only". This
> > > > >> >> is
> > > > >> different
> > > > >> >> from "disabled" state, like I explained in a
> previous email.
> > > > >> >>
> > > > >> >> 2. I removed some redundant skin hooks - I can
> send you the
> > > > >> >> updated
> > > > >> list
> > > > >> >> of selectors I am using. I also couldn't get the
> > > > >> >> "pass-through
> > > > >> states"
> > > > >> >> :visited, :active and :unvisited to work, just as
> > > you. So I have
> > > > >> >> temporarily defined selectors like Jeanne
> suggested (using
> > > > >> >> p_AFVisited, p_AFUnvisited etc. and renamed :active to
> > > > >> >> :selected).
> > > > >> >>
> > > > >> >> 3. Finally I have simplified the rules for
> > > determining the state
> > > > >> >> of joins.
> > > > >> >> I figured it would be much simpler if we did the
> > > following. The
> > > > >> >> join to the left of a stop, is 'always in the same
> > > state as the
> > > > >> >> stop' (Overflows could also follow the same rules as
> > > stops). So
> > > > >> >> for instance for a train
> > > > >> like
> > > > >> >>
> > > > >> >> V ----- VR ----- UV ----- A ----- D ----- UVR ----- V
> > > > >> >> vr uv v d uvr v
> > > > >> >>
> > > > >> >>
> > > > >> >> NOTE: The only exception, is the join to the left of an
> > > > >> >> active
> > > > >> stop is
> > > > >> >> visited. Also, UVR and VR are stops that are in 2 states
> > > > >> >> simulataneously - 'visited & read-only' and 'unvisited &
> > > > >> >> read-only'. Read-only implies
> > > > >> the
> > > > >> >> stop cannot be reached (and hence not clickable) and
> > > is dictated
> > > > >> by the
> > > > >> >> 'readOnly' property on the component
> commandNavigationItem.
> > > > >> >>
> > > > >> >> Please let me know if the above is ok.
> > > > >> >>
> > > > >> >>
> > > > >> >> Thanks
> > > > >> >> - Pavitra
> > > > >> >>
> > > > >> >>
> > > > >> >> > -----Original Message-----
> > > > >> >> > From: Simon Lessard [mailto:[EMAIL PROTECTED]
> > > > >> >> > Sent: Monday, August 28, 2006 11:43 AM
> > > > >> >> > To: [email protected]
> > > > >> >> > Subject: Re: Train selectors
> > > > >> >> >
> > > > >> >> > Hmmm you mean somthing like af|train::stop.p_AFVisited?
> > > > >> >> >
> > > > >> >> > On 8/28/06, Jeanne Waldman
> > > <[EMAIL PROTECTED]> wrote:
> > > > >> >> > >
> > > > >> >> > > I was thinking :selected for :active.
> :selected could be
> > > > >> >> > > used
> > > > for
> > > > >> >> > > other components, too.
> > > > >> >> > > For :visited/:unvisited, I can't think of a
> better name.
> > > > >> >> > I'm thinking
> > > > >> >> > > that we should use .p_AFVisited,
> .P_AFUnvisited until we
> > > > >> >> > > have
> > > > the
> > > > >> >> > > pseudo-class support in. These wouldn't in a
> public api
> > > > >> >> > format, though.
> > > > >> >> > >
> > > > >> >> > > - Jeanne
> > > > >> >> > >
> > > > >> >> > > [EMAIL PROTECTED] wrote:
> > > > >> >> > >
> > > > >> >> > > >Hello,
> > > > >> >> > > >
> > > > >> >> > > >I thought about the following name changes for
> > > the selectors:
> > > > >> >> > > >
> > > > >> >> > > >:active --> :current or :selected
> > > :visited/:unvisited -->
> > > > >> >> > > >:completed/:uncompleted or
> > > > :seen/:unseen
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >Do you have any other idea/preference?
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >Regards,
> > > > >> >> > > >
> > > > >> >> > > >~ Simon
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >"Simon Lessard" <[EMAIL PROTECTED]>
> > > > >> >> > > >2006-08-25 22:49
> > > > >> >> > > >Please respond to adffaces-dev
> > > > >> >> > > >
> > > > >> >> > > > To: "Trinidad - Dev"
> > > > >> >> > <[email protected]>
> > > > >> >> > > > cc:
> > > > >> >> > > > Subject: Train selectors
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >Yes... again...
> > > > >> >> > > >
> > > > >> >> > > >I made a new renderer and it work quite well,
> > > but I had to
> > > > >> >> > > >use :ora-visited and :ora-active for some
> > > selectors because
> > > > >> >> > > >those
> > > > >> are
> > > > >> >> > > >"pass through"
> > > > >> >> > > >values.
> > > > >> >> > > >Anyone have better name suggestion while we
> > > implement state
> > > > >> >> > > >interception on a per component basis?
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >Regards,
> > > > >> >> > > >
> > > > >> >> > > >~ Simon
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > >
> > > > >> >> > >
> > > > >> >> >
> > > > >> >> >
> > > > >> >> >
> > > > >> >>
> > > > >> >>
> > > > >> >
> > > > >>
> > > > >>
> > > > >
> > > >
> > > >
> > >
> > >
> > >
> >
> >
>
>
>