I'm ok with the highlight of the active filters. Active states are marked
with highlightColor. Even tags in the Tag Cloud, when active, are
highlighted with that color, also the table headers when they have an active
sorting.
What I don't understand is why you think you need to add another color to
the Color Theme class. If you think the color is too light then the client
should modify the color in his Color Theme to add more contrast with his
theme.
There is a process to enhance the actual Color Theme variables. There is a
page on incubator
where is a proposal for a new Light Background Color and also a study of the
colors we would need for the Error/Notifications Messages.
On Thu, Apr 22, 2010 at 14:20, Jerome Velociter <jerome(a)xwiki.com> wrote:
   On Wed, Apr
21, 2010 at 12:35, Jerome Velociter <jerome(a)xwiki.com> 
 wrote:
  Hello Denis,
  Hi devs,
 We have a customer request for highlighting the filters that are 
 currently
  applied on a livetable. The rationale of this, is
that sometimes the
 filtered table is empty or unexpected by the user, but he does not 
 notice
  that a filter is currently applied. We have
written a proposed
 implementation as well and you may also see what it looks like in the
 attached screenshot, for both text field and select box (for webkit).
 Currently, I choose yto use the "background highlight color" of the 
active
  color theme, but my feeling is that adding
another highlight color 
 would
  be
 better. 
 I agree 
 Well, since I have not followed what have been discuss regarding the 
  color
  theme, what would be the process to add a new
color in it ? How do we
 expect
 to manage the evolution of the color theme to avoid it to became 
 cluttered
  ? 
 I don't know either :)
  Should we open a discussion on that if none have
been already ? 
 Caty, Sergiu ? Do we have a process defined for this ?
 
 I have also hook to events separately from the "main" refresh handler,
 because I do not understand the implementation of this handler. Why 
 using
  an
 intermediate makeRefreshHandler, and why it does not properly provide 
 the
  currently changed filter to the handler ? Could
or should I change 
 that ?
 Yes you can. The refresh handler should probably not return a function
 but
 be a livetable function that is bound as event listener, i.e :
 this.handleRefresh.bindAsEventListener(this)
 versus the current
 this.makeRefreshHandler(this)
 This way you will be able to access the event from the handler, thus the
 filter that triggered it.
 
 
 Ok, I will adapt my implementation and the current code.
  One last thing : you should not need the try {}
catch {} statements in
 your code. I don't see where it could fail (and if it could, I think
 it's
 better to try to fix it so that it does not happen). Try catch blocks
 IMO
 should be used only in very specific occasions (for example when you
 know
 code can fail under certain circumstances, but you don't want to stop
 the
 execution). Anyway that's just my opinion, we've never stated on this as
 a
 best practice, but we can discuss it in another thread if needed. I'd be
 curious to get input from Sergiu about this for example.
 
 I agree. My habits was that minor JS function (those that do not impact
 usability) should never ever crash, so I usually protect them with
 try/catch
 in all cases. This is maybe overkill here. Should we also open a 
  discusion
  on this best practice ? Sergiu ? 
 We already have some JS best practices documented at
 
http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HJavaScr…
 ,
 but it's probably about time we discuss again and expand those with new
 ones. In addition of try/catch discussion, I can think of code style,
 documentation style and preferred module pattern style.
 Jerome
 Denis
 --
 Denis Gervalle
 SOFTEC sa - CEO
 eGuilde sarl - CTO
 _______________________________________________
 devs mailing list
 devs(a)xwiki.org
 
http://lists.xwiki.org/mailman/listinfo/devs
 
 _______________________________________________
 devs mailing list
 devs(a)xwiki.org
 
http://lists.xwiki.org/mailman/listinfo/devs