New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tablesorter theme doesn't use same colors as before #544

Closed
ocean90 opened this Issue Sep 13, 2016 · 10 comments

Comments

Projects
None yet
2 participants
@ocean90
Member

ocean90 commented Sep 13, 2016

Before:

before

## After:

after

## ToDo: - [x] Blue highlight color, but only when a colum sort was changed. - [x] Background color of `.odd td` should be white. - [x] The border between table cells should be restored. - [x] There should be no [flickering](https://cloud.githubusercontent.com/assets/617637/18563699/fac1cd66-7b88-11e6-9f18-0220273c6599.gif) after Tablesorter has initialized the table.
@ocean90

This comment has been minimized.

Member

ocean90 commented Sep 15, 2016

@toolstack Sorry, I wasn't clear enough. I've updated the description with a todo list.

@toolstack

This comment has been minimized.

Contributor

toolstack commented Sep 15, 2016

I would argue that "only when a column sort was changed" is a bug in the old code. The current sort column should be highlighted at all times. I have to admit I didn't even notice you could sort by column until I was looking through the code one day and noticed the table sorter script :)

The background colour of the odd rows should probably not be white any more as the new table sorter code highlights the row that the cursor is over in white. We could change that to another colour though or remove it of course, but it seems like a good feature overall.

I'll add the cell boards back in.

The flickering seems to be chrome specific, I'll see if I can track it down.

@toolstack

This comment has been minimized.

Contributor

toolstack commented Sep 15, 2016

Dug in to the chrome issue and it turns out the code doesn't support chrome (see here).

By manuall adding the various classes the flicker can be significantly reduced, but the columns still "bounce" (grow and then shrink back) once during table init... sometimes. Other times it seems to be fine.

@toolstack

This comment has been minimized.

Contributor

toolstack commented Sep 15, 2016

Some more poking eliminates the table sorter from the last bit of the flickering. It looks like Chrome has a bug in it's rendering engine that miscalculates the column width during the first pass and then "fixes" it on a second pass.

@toolstack

This comment has been minimized.

Contributor

toolstack commented Sep 16, 2016

Confirmed the "bounce" happens in previous versions of GP, so not related to the table sorter update.

The only way it looks like to fix it is to set a fixed width for the table that is greater than the sum of the row requirements. For example if you set the table width from auto to 600px (note 600px is dependent on the locale list and display properties. On my setup the table requires at minimum 536px to display everything so any number above that would work).

Or setting it to 100% works as well but of course looks strange with so few columns.

@ocean90

This comment has been minimized.

Member

ocean90 commented Sep 17, 2016

I would argue that "only when a column sort was changed" is a bug in the old code. The current sort column should be highlighted at all times.

The current blue is quite distracting, that's why I suggested to apply this only when a user changes it. I think we should remove the background and make the arrows more prominent instead. They should be white as a first step.

The background colour of the odd rows should probably not be white any more as the new table sorter code highlights the row that the cursor is over in white.

OK, let's wait for the redesign…

Dug in to the chrome issue and it turns out the code doesn't support chrome.

Well, the plugin is quite old, but nice that we support IE 6 at least. Maybe we should have searched for an alternative instead of updating it in #502.

@toolstack

This comment has been minimized.

Contributor

toolstack commented Sep 17, 2016

The current blue is quite distracting, that's why I suggested to apply this only when a user changes it. I think we should remove the background and make the arrows more prominent instead. They should be white as a first step.

I agree with the white arrows, I'll update the css. Let's change the highligh color to something less distracting. We could go with a darker grey or (my preference) the GP purple.

Well, the plugin is quite old, but nice that we support IE 6 at least. Maybe we should have searched for an alternative instead of updating it in #502.

I assumed we'd wait for the redesign on replacing the current table sorter, 502 grew out of the javascript fixes and minification work.

@toolstack

This comment has been minimized.

Contributor

toolstack commented Sep 17, 2016

Some sample options:

sort-highlights

#0026FF Dark Blue
#000000 Black
#32373C GP Base Gray
#826EB4 GP Purple
#3399FF Light Blue

I think black actually makes the most sense, it's unobtrusive and it's the highlight we use for the top level menu as well.

@ocean90

This comment has been minimized.

Member

ocean90 commented Sep 17, 2016

Thanks for graphic. Let's use 32373C.

@toolstack

This comment has been minimized.

Contributor

toolstack commented Sep 17, 2016

Done.

@ocean90 ocean90 closed this in #546 Sep 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment