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

fix(TableContainer): no space on empty foot + clip scrollbar #1827

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

tujoworker
Copy link
Member

@tujoworker tujoworker commented Dec 19, 2022

Based on UX wish, this is how it looks with this change:
Screenshot 2022-12-19 at 10 47 33

and when empty:

Screenshot 2022-12-19 at 10 47 45

@tujoworker tujoworker requested a review from langz December 19, 2022 07:33
@gatsby-cloud
Copy link

gatsby-cloud bot commented Dec 19, 2022

✅ DNB Eufemia Portal deploy preview ready

@tujoworker tujoworker force-pushed the fix/table-container-scroll branch 2 times, most recently from f179eba to 939bd3f Compare December 19, 2022 08:14
@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 19, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 57a45de:

Sandbox Source
eufemia-starter Configuration

Copy link
Contributor

@langz langz left a comment

Choose a reason for hiding this comment

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

Fancy fancy 💃

So we only want to clip the scrollbar when using the TableContainer, and not for all tables?

I tried testing this PR on my phone, iPhone, and as we discovered on Friday, I was not able to see any scrollbars at all for any of our tables 🤔

@tujoworker tujoworker force-pushed the fix/table-container-scroll branch from 939bd3f to 827a24e Compare December 19, 2022 09:03
@langz
Copy link
Contributor

langz commented Dec 19, 2022

An other observation, which I don't think is directly related to this PR, but something else related to scrolling(which don't exists on https://eufemia.dnb.no/ yet).

When opening this deploy preview on my phone using Safari, then opening the menu, it looks to be some issues maybe related to https://github.com/dnbexperience/eufemia/pull/1814/files#diff-f0adfb69cb45eb94b8016448bc17488f3e2ba81514c694b5ce0f34f4e245bc56R290 ?

image

@tujoworker
Copy link
Member Author

For the TableContainer the scorllbar is inside, else it is outside.

Screenshot 2022-12-19 at 10 09 45

@tujoworker
Copy link
Member Author

it looks to be some issues maybe related to

ohh, yes! good catch!

@tujoworker tujoworker force-pushed the fix/table-container-scroll branch 3 times, most recently from b07067b to 7d69252 Compare December 19, 2022 10:11
@tujoworker tujoworker force-pushed the fix/table-container-scroll branch from 7d69252 to 57a45de Compare December 19, 2022 10:11
@tujoworker tujoworker merged commit 04dd93b into main Dec 19, 2022
@tujoworker tujoworker deleted the fix/table-container-scroll branch December 19, 2022 10:54
tujoworker pushed a commit that referenced this pull request Dec 19, 2022
# [9.43.0](v9.42.0...v9.43.0) (2022-12-19)

### Bug Fixes

* **Button:** support target property ([#1817](#1817)) ([71f5ee3](71f5ee3))
* **TableContainer:** align always visible scrollbar ([#1819](#1819)) ([c16519d](c16519d))
* **TableContainer:** no space on empty foot + clip scrollbar ([#1827](#1827)) ([04dd93b](04dd93b))
* **Table:** prevent expanded row to not change column width ([#1816](#1816)) ([bbf94a8](bbf94a8))
* **useHandleSortState:** change default direction from asc to off ([#1826](#1826)) ([c7e4d6f](c7e4d6f))
* **useHandleSortState:** fix import issue ([#1823](#1823)) ([c595ff1](c595ff1))

### Features

* **Scrollbar:** add new look to scrollbars ([#1815](#1815)) ([a590b50](a590b50))
@tujoworker
Copy link
Member Author

🎉 This PR is included in version 9.43.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants