-
Notifications
You must be signed in to change notification settings - Fork 304
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
NAS-117474 / 22.12 / Datasets table header sticky #7091
NAS-117474 / 22.12 / Datasets table header sticky #7091
Conversation
@denysbutenko @undsoft - please have a look on these updates: Screen.Recording.2022-09-16.at.14.39.27.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…re/NAS-117474-datasets-table-header-sticky
Codecov Report
@@ Coverage Diff @@
## master #7091 +/- ##
==========================================
+ Coverage 41.15% 41.88% +0.72%
==========================================
Files 1045 1049 +4
Lines 46076 45400 -676
Branches 6395 6216 -179
==========================================
+ Hits 18964 19017 +53
+ Misses 27112 26383 -729
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same issue
Could not find solution as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -318,3 +311,8 @@ $padding-left: 8px; | |||
} | |||
} | |||
} | |||
|
|||
ix-tree { | |||
max-height: calc(100vh - 330px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What stands for 330px / How is it gained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just calculated it visually, I think it will be good to refactor datasets blocks, to have same height via flex somehow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we like that approach I found, I'll work on refactoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous option was better. We wanted to keep page scrolling and not just have a smaller area to scroll.
@undsoft - reverted to previous solution |
@undsoft @denysbutenko Screen.Recording.2022-09-21.at.19.28.29.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scrolling works perfectly now, nice work. 👍🏻
Need to revert changes to card widths to fix them.
src/app/pages/datasets/components/dataset-management/dataset-management.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird enough both times when I was testing this PR, I had to remove node_modules
and reinstall packages. Just doing yarn
would result in compilation error, not sure why.
May be worth doing the same and committing changed yarn.lock
.
But generally works well.
No description provided.