Skip to content
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

More readable tables and lists: Alternate row background colors #4230

Merged
merged 2 commits into from
Jun 4, 2020

Conversation

cd2357
Copy link
Contributor

@cd2357 cd2357 commented May 3, 2020

Alternate row background colors are now available, regardless of theme.

A few CSS selectors affecting table and list row coloring were moved the dark theme stylesheet (theme-dark.css) to the common stylesheet (bisq.css).

These selectors are theme-independent, since they re-use variables defined in each theme stylesheet (like -bs-background-color). Therefore a more appropriate place for them is in the common stylesheet.

This move means that alternating row background colors are now available for all tables and lists, in all views, in both dark and light themes.

cd2357 added 2 commits May 3, 2020 14:09
…rdless of theme

A few CSS selectors affecting table and list row coloring were moved the dark theme stylesheet (theme-dark.css) to the common stylesheet (bisq.css).

These selectors are theme-independent, since they re-use variables defined in each theme stylesheet (like -bs-background-color). Therefore a more appropriate place for them is in the common stylesheet.

This move means that alternating row background colors are now available for all tables and lists, in all views, in both dark and light themes.
The same styling as for a selected row, is applied for a hovered row.

This makes the UI more intuitive, helping users better navigate through and focus on specific rows, especially in large tables.
@cd2357 cd2357 changed the title More readable tables and views: Table rows alternate colors More readable tables and views: Alternate row background colors May 3, 2020
@cd2357 cd2357 changed the title More readable tables and views: Alternate row background colors More readable tables and lists: Alternate row background colors May 3, 2020
@sqrrm sqrrm added the in:gui label May 5, 2020
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add screenshots.

@cd2357
Copy link
Contributor Author

cd2357 commented May 6, 2020

Here are some screenshots.

Before:

Screenshot from 2020-05-06 22-03-27

Screenshot from 2020-05-06 22-04-27

After:

Screenshot from 2020-05-06 21-56-48

Screenshot from 2020-05-06 21-57-07

@ghost
Copy link

ghost commented May 10, 2020

For "not full tables" it look in my opinion little bit akward

image

Maybe not to use stripes for empty rows?

@@ -867,6 +891,32 @@ textfield */
* Table *
* *
******************************************************************************/
.table-view .table-row-cell:even .table-cell {
-fx-background-color: derive(-bs-background-color, 5%);
-fx-border-color: derive(-bs-background-color,5%);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe nitpicky, but there is whitespace inconsistency :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, just run some auto formatter on the code to get spacing uniform. It's a small thing but someone will do it later and there will be changes in an unrelated PR.

Apart from that I'm good to merge this PR.

@ghost
Copy link

ghost commented May 10, 2020

I would personally make it less "sharp" by lowering +-5% to just +-3%

Here is how it looks like with 3. (Note: I have also found a UI glitch that lines have different size (probably only problem when they are empty.)

image

@ghost
Copy link

ghost commented May 10, 2020

Concept ACK

@cd2357
Copy link
Contributor Author

cd2357 commented May 18, 2020

For "not full tables" it look in my opinion little bit akward

Maybe not to use stripes for empty rows?

Would potentially make sense, but it's not related to my change.

To achieve that, table model logic would have to be changed to remove specific CSS classes from rows if they are empty. That' most likely custom java logic that has to be done for every table.

This PR is only about bringing a dark-theme behavior (showing odd/even rows as having different colors) also into the light-theme.

So I would say, to keep PRs conceptually separated, that change should be covered in a different PR (especially since it affects both dark and light themes -- this PR only fixes some CSS tags in the light theme).

@cd2357
Copy link
Contributor Author

cd2357 commented May 18, 2020

I would personally make it less "sharp" by lowering +-5% to just +-3%

Yes, would be worth exploring.

But perhaps in a separate PR (see comment above).

I tried to keep this PR specifically about bringing a dark-theme-only behavior (alterate row colors) to both dark and light themes.

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@sqrrm sqrrm added this to the v1.3.5 milestone Jun 4, 2020
@sqrrm sqrrm merged commit 06d6953 into bisq-network:master Jun 4, 2020
@ripcurlx
Copy link
Contributor

ripcurlx commented Jun 10, 2020

In that case it would have been great to get @pedromvpg's feedback before merging, as he did this by intent as far as I remember. Anyways I'll leave it in for this release and we might make an adaption in the next release.

@cd2357 cd2357 deleted the table-rows-alternate-colors branch August 20, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants