Skip to content

[Bug] mobile sheet bar container does not display normally because of scrollbar #2969

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

Closed
2 tasks done
universeroc opened this issue Aug 8, 2024 · 7 comments · May be fixed by #2970
Closed
2 tasks done

[Bug] mobile sheet bar container does not display normally because of scrollbar #2969

universeroc opened this issue Aug 8, 2024 · 7 comments · May be fixed by #2970
Labels

Comments

@universeroc
Copy link

Before you submit this issue, have you checked the following

  • Is this really a problem?
  • I have searched the Github Issues for similar issues, but did not find anything.

Affected packages and versions

0.2.6

Reproduction link

https://stackblitz.com/~/github.com/awesome-univer/sheets-vite-demo

Expected behavior

image

Actual behavior

image

System information

No response

@universeroc universeroc added the bug Something isn't working label Aug 8, 2024
@wzhudev
Copy link
Member

wzhudev commented Aug 8, 2024

Have you tried to open this feature in the dev tool? Since the demo is for mobile, you should toggle it open.

image

Your PR seems impropriate because it would make the sheetbar unscrollable.

@universeroc
Copy link
Author

Have you tried to open this feature in the dev tool? Since the demo is for mobile, you should toggle it open.

image

Your PR seems impropriate because it would make the sheetbar unscrollable.

Yes, I did, all the same. :)

@universeroc
Copy link
Author

Have you tried to open this feature in the dev tool? Since the demo is for mobile, you should toggle it open.

image

Your PR seems impropriate because it would make the sheetbar unscrollable.

Yes, my PR just remove the overflow-x attribute to make the main content to show normally because I don't think a horizonal scrollbar is a good design for users not only on desktop but also on mobile.

So do you have a better solution to solve it? :)

@wzhudev
Copy link
Member

wzhudev commented Aug 9, 2024

Have you tried to open this feature in the dev tool? Since the demo is for mobile, you should toggle it open.
image
Your PR seems impropriate because it would make the sheetbar unscrollable.

Yes, I did, all the same. :)

Please provide detailed information of how to reproduce that.

@wzhudev
Copy link
Member

wzhudev commented Aug 9, 2024

Have you tried to open this feature in the dev tool? Since the demo is for mobile, you should toggle it open.
image
Your PR seems impropriate because it would make the sheetbar unscrollable.

Yes, my PR just remove the overflow-x attribute to make the main content to show normally because I don't think a horizonal scrollbar is a good design for users not only on desktop but also on mobile.

So do you have a better solution to solve it? :)

It is your personal flavor. I don't see why this is necessary to change the code. You can overrite it with CSS in your project, or write a plugin that provides UI you want.

@wzhudev wzhudev added invalid and removed bug Something isn't working labels Aug 9, 2024
@universeroc
Copy link
Author

@universeroc
Copy link
Author

universeroc commented Aug 10, 2024

Have you tried to open this feature in the dev tool? Since the demo is for mobile, you should toggle it open.
image
Your PR seems impropriate because it would make the sheetbar unscrollable.

Yes, my PR just remove the overflow-x attribute to make the main content to show normally because I don't think a horizonal scrollbar is a good design for users not only on desktop but also on mobile.
So do you have a better solution to solve it? :)

It is your personal flavor. I don't see why this is necessary to change the code. You can overrite it with CSS in your project, or write a plugin that provides UI you want.

OK, it's ok I'll solve it in my way.

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 a pull request may close this issue.

2 participants