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.

Sign up for GitHub

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

Reducing the width of "Punch" button to leave room for the scrollbars #455

Merged
merged 2 commits into from
Oct 13, 2020
Merged

Reducing the width of "Punch" button to leave room for the scrollbars #455

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff878cc
Reducing the width of "Punch" button to leave room for the scrollbars
ccsCoder Oct 12, 2020
ec0fcf2
Increasing scrollbar width as per review comments
ccsCoder Oct 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ html[data-theme="cadent-star"] {
/* There are some libraries that impose their own colors. These may need to be overriden for specific themes */

::-webkit-scrollbar {
width: 10px;
height: 10px;
width: 5px;
height: 5px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a feeling 5px is too small... Not sure if it's just me.
@araujoarthur0 what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looked small on the other PR...
Is there a reason we are avoiding having the scrollbar only on the table part ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As per @ccsCoder comment on the issue, this is not trivial right now. And from what I remember, I agree.

@ccsCoder, do you think we can keep it width at least 8 or 10 for the scroll bar, and take a little bit more pixels from the button?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In detail, if we want to have scrolling on the table, the right way to do it would be to have it on the tbody because we'd want to keep the Table Headers fixed.
The only way to do that (while still using <table and not divs) tag would be to have both <thead> and <tbody as display: block and have overflow set on them.

This works ( i've verified it ) but the "Punch button" overlaps the table. This is due to the fact that it's position is fixed which breaks it from the page layout flow. There are ways to get around that but the best way would be to change the markup in a way as follows ->

--- Title---
--- header ---
[[[ table ]]]. ---> expands to fill the available space.
-- footer---

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thamara How does this look ? Scrollbar is 8px wide here.

Screen Shot 2020-10-13 at 8 13 19 AM

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the great explanation.
Given that a lot of changes would be required and that the future of the fixed table is still under discussion, I guess we can go for the workaround right now.
But if you'd like to, you could open an issue for the flexible table. I suppose the same issue must be happening there, though it most likely is not a pure table element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

}

::-webkit-scrollbar-thumb {
Expand Down Expand Up @@ -385,7 +385,7 @@ body {

.punch-button {
opacity: 0.9;
width: 100%;
width: calc(100vw - 40px);
height: 100%;
border: none;
color: var(--punch-color);
Expand Down