23 Apr
2010
23 Apr
'10
7:57 a.m.
On Thu, Apr 22, 2010 at 21:44, Sergiu Dumitriu <sergiu(a)xwiki.com> wrote:
it's
try/catch
Agreed also, this is not the intend here, my intend was just to avoid any
"unexpected" issue from a minor feature.
I will remove them since there no reason for it fails.
Note that the keyup handler is not the best option IMO. It would fail if
On 04/22/2010 09:29 AM, Denis Gervalle wrote:
Well, since I have not followed what have been discuss regarding the color
I completely agree with your analysis. The problem here is that highlight is
also used for sorting column, and IMO this is confusing or not so much
highlighted when the same column is filtered and sorted. But this is matter
of taste anyway. If most you say the highlight color is enough, than I will
use it.
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 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 ?
Should we open a discussion on that if none have
been already ?
Color themes should be kept small, so that it's easy to design a new
color theme, and it keeps things in harmony. And, another aspect is that
different elements of the wiki should use similar colors, so that the UI
is familiar. We have a highlight color so that users can remember it and
recognize a highlight no matter where it appears. And I really don't
think that a skin that uses all the colors of the rainbow would look
nice and would really be useful. Thus, when designing the components of
the color theme, we tried to use as few as possible, while still
allowing creativity. When a color is not enough, remember that you can
still create variations of it by playing with the opacity (this is what
we did for the poll to simulate disabled colors).
>>
>> 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 > 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 in all cases. This is maybe overkill here. Should
we also open a
discusion
on this best practice ? Sergiu ?
I agree that JS code should genrally not use try catch. I prefer to
check the data (if condition holds true, do this) than to assume the
data is there and is valid, and watch out for exceptions. One advantage
of pre-data checks is that partial code can't fail midway, while a
try-catch must cleanup in the end.
the user copy-pastes text using the mouse.
Additionally, the keyup is
executed too often, maybe onchange would be enough, although this means
that the highlight won't be applied until the use moves the focus out of
the field.
Following Jerome remarks, I have cleanup the refresh handler used for filter
and use for all, so that highlighting is clearly linked with the filtering
its self. Currently, filtering use the "keyup" event for input type=text to
trigger the refresh of the table, this has issues you mention, you are
invited to file a JIRA for that :), this is not the goal of my proposal to
do so.
Maybe a better idea would be to:
- apply highlight onfocus, even if the input is empty, and keep it that
way as long as the input still has focus
- onblur decide if the highlight needs to be kept or not
Why not, but why bother while on the same event we do ajax and many heavier
stuffs... than just updating a style ? I also find better to be tightly
linked with the real refresh, so pasting with the mouse currently does not
highlight nor it trigger the refresh and the real filtering.
If nobody see any more issue, I would like commit the attached patch in 2.4,
should this still deserve a vote ?
Denis
--
Denis Gervalle
SOFTEC sa - CEO
eGuilde sarl - CTO