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

Makes planning actions sticky #378

Conversation

laurentChin
Copy link
Contributor

Closes #377

@@ -75,3 +75,15 @@ table.availability-table {
color: white;
}
}

@media screen and (min-width: 1001px) {
Copy link
Contributor

@mRoca mRoca May 3, 2020

Choose a reason for hiding this comment

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

Why are you updating the user availability table, as this PR is related to the planning page ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I put the rules in the wrong place, fix in in progress.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those rules are now removed, all the rules needed are in planning.scss.

@@ -169,3 +169,15 @@ $tableBoxSize: 40px;
}
}
}

@media screen and (min-width: 1001px) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Those values should be the default one, and you should only use media query for the mobile versions

@media screen and (min-width: 1001px) {
.planning-actions-container {
position: sticky;
top: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you hiding all the table sticky headers ?

position: sticky;
top: 0;
z-index: 1;
width: 100vw;
Copy link
Contributor

Choose a reason for hiding this comment

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

you must put a size to the buttons container, as this line will create very large buttons (you can have a look to the screenshot I've put on your issue)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this width is not required for this PR, it's just to have the form actions container beeing not "floating" into the page but that's a cosmetic issue, the real need is to have the actions constantly accessible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please have a look to the screenshot I've put in the issue: the table headers are hidden, and buttons are now too large

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I've seen the screenshot the PR is just not up to date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The table headers are fixed, Chrome doesn't apply the shift on top value on thead it has to be set on th cells.
I have add a .planning-actions-container-wrapperdiv to keep a background on the actions and prevent them to be "floating" in the page without removing the bootstrap sizing on action buttons.

@mRoca
Copy link
Contributor

mRoca commented May 3, 2020

image

It would be nice to force the div to be sticky on the left as well

@laurentChin laurentChin force-pushed the feat/makes-planning-actions-sticky branch from ab52e4f to 5e90df0 Compare May 3, 2020 21:30
@mRoca
Copy link
Contributor

mRoca commented May 4, 2020

I've a new issue on Chrome: the div is now fixed on the left, but if I only display 2 or 3 days, I can scroll on the right event if the page is fully displayed. When "scrolling" on the right, the page and and the horizontal scrollbar are growing at the same time.

It's weird, I can make a video if you need it to understand the problem

@laurentChin
Copy link
Contributor Author

I've a new issue on Chrome: the div is now fixed on the left, but if I only display 2 or 3 days, I can scroll on the right event if the page is fully displayed. When "scrolling" on the right, the page and and the horizontal scrollbar are growing at the same time.

It's weird, I can make a video if you need it to understand the problem

I did not succeed to reproduce this bug but I've had a constraint to prevent the floating behavior to be triggered if the table width is smaller than the window, I should've done this at the beginning.
Tell me if the bug is still here despite this constraint.

@laurentChin laurentChin force-pushed the feat/makes-planning-actions-sticky branch from 2bda228 to 1360d58 Compare May 5, 2020 17:44
@@ -2,8 +2,18 @@

$tableBoxSize: 40px;

.planning-actions-container-wrapper {
width: 100vw;
Copy link
Contributor

Choose a reason for hiding this comment

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

This line forces the page to always have an horizontal scrollbar event with only one displayed day

@mRoca
Copy link
Contributor

mRoca commented May 5, 2020

There is still the bug, but with bigger screen. In order to reproduce:

  • On Chrome (Chromium 79 on my computer)
  • Go to the planning
  • Display like 4 or 5 days
  • Scroll to the right border with your touchpad (not the mouse)
  • Scroll again to the right, you will see the window growing

=> I can show you on Discord, by sharing my screen

@laurentChin
Copy link
Contributor Author

That's ok I can reproduce it on Chrome Windows with Browserstack, on Mac OS the scroll is handled differently.

@mRoca
Copy link
Contributor

mRoca commented May 11, 2020

Hi @laurentChin , any update?

@laurentChin
Copy link
Contributor Author

Hi @mRoca, no update at this point I’m not satisfied with any of my fixes, they are over-complicated for the behavior I expect, so I’m gonna rewrite the initial implementation to make it more elegant. I just need to find some time to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Makes planning actions sticky
2 participants