On Wed, Apr 21, 2010 at 12:35, Jerome Velociter <[email protected]> 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 ? Should we open a discussion on that if none have been already ?

...

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 ? Denis -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO

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 http://incubator.myxwiki.org/xwiki/bin/view/Improvements/ColorThemesExtensio... 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 <[email protected]> wrote:

...

On Wed, Apr 21, 2010 at 12:35, Jerome Velociter <[email protected]> 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#HJavaScri... , 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 [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On 04/22/2010 09:29 AM, Denis Gervalle wrote:

On Wed, Apr 21, 2010 at 12:35, Jerome Velociter<[email protected]> 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 ? 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 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 ?

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. -- Sergiu Dumitriu http://purl.org/net/sergiu/

On 04/23/2010 07:57 AM, Denis Gervalle wrote:

On Thu, Apr 22, 2010 at 21:44, Sergiu Dumitriu<[email protected]> wrote:

...

On 04/22/2010 09:29 AM, Denis Gervalle wrote:

...

On Wed, Apr 21, 2010 at 12:35, Jerome Velociter<[email protected]> 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 ? 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 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.

...

...

...

...

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 ?

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.

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

...

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 ?

No, go ahead. -- Sergiu Dumitriu http://purl.org/net/sergiu/

On Wed, Apr 21, 2010 at 12:43, Caleb James DeLisle <[email protected]

wrote:

You can change it just for the customer by using a custom stylesheet with skin extension. LiveValidation used to turn the borders red on the bad fields using CSS but I removed it because it was unfriendly with the Toucan skin. See revision: http://www.mail-archive.com/[email protected]/msg14131.html

I am not sure I get your point. This is not only a stylesheet change, since I need the field to be styled by the livetable JS code. This is more a matter of, do you agree that it is useful to highlight applied filters in livetable ? what do you think of the implementation ? and what do you think of adding a new color in the color theme for highlighting since using the standard one is not the best option IMO ? Denis -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO