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

[Bug]: Missing breadcrumb ellipsis -> Grid view button pushed out of view #46635

Closed
4 of 8 tasks
ChristophWurst opened this issue Jul 19, 2024 · 6 comments · Fixed by #46865
Closed
4 of 8 tasks

[Bug]: Missing breadcrumb ellipsis -> Grid view button pushed out of view #46635

ChristophWurst opened this issue Jul 19, 2024 · 6 comments · Fixed by #46865

Comments

@ChristophWurst
Copy link
Member

ChristophWurst commented Jul 19, 2024

⚠️ This issue respects the following points: ⚠️

Bug description

Long breadcrumbs are not limited in length. They can take up too much space:

Bildschirmfoto vom 2024-07-19 13-15-59

Steps to reproduce

  1. Create a deep hierarchy with long folder names
  2. Navigate to it

Expected behavior

Responsive design

Installation method

None

Nextcloud Server version

29

Operating system

None

PHP engine version

None

Web server

None

Database engine version

None

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

@ChristophWurst ChristophWurst added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap feature: files regression 29-feedback labels Jul 19, 2024
@skjnldsv
Copy link
Member

Looks more like a component issue? @raimund-schluessler

@raimund-schluessler
Copy link
Member

I would say the component works as intended, but it's not used "correctly". The breadcrumbs component takes as much space as it is given, which means its container needs to be sized adequately. Currently, .files-list__breadcrumbs (the breadcrumbs top element), is instructed to take 100 % of the parents width, which pushes the grid view button out. Applying width: calc(100% - 64px); (taking into account the button size and the additional margin of 10 px) would fix this issue, but it feels a bit "hacky". Adjusting the flex layout would probably be better. But see here anyway:

Without the calc:
Screenshot 2024-07-29 at 09-10-10 Dateien - Nextcloud

And with width: calc(100% - 64px);:
Screenshot 2024-07-29 at 09-10-33 Dateien - Nextcloud

@skjnldsv
Copy link
Member

skjnldsv commented Jul 30, 2024

Not really, we are using automated flex. Width 100% is expected as flex-shrink is enabled.
image

The breadcrumbs are designed (in Files), to take all the remaining space after all elements are layed out. The Buttons have a flex shring/grow set to 0. Only the breadcrumbs is allowed to resize. That is the clean way. Using calc is not :)
image

@raimund-schluessler
Copy link
Member

I know calc is not the way, that's what I meant with "hacky" 😉

However, the flex layout does not work properly here, for some reason. The breadcrumbs container has a 100% parent width without shrinking or respecting its siblings.

@raimund-schluessler
Copy link
Member

@skjnldsv I had a look again. The problem why the breadcrumbs container .files-list__breadcrumbs does not shrink, is that the flex specification states to not shrink a flex item below the minimum size of its child, see https://www.w3.org/TR/css-flexbox-1/#min-size-auto and https://www.bigbinary.com/blog/understanding-the-automatic-minimum-size-of-flex-items.

We either have to set min-width: 0; (or any other reasonable value) or overflow: hidden; for the breadcrumbs container .files-list__breadcrumbs here:

.files-list__breadcrumbs {
// Take as much space as possible
flex: 1 1 100% !important;
width: 100%;
height: 100%;
margin-block: 0;
margin-inline: 10px;

grafik

Result:
grafik

@skjnldsv Is that fine for you?

@skjnldsv
Copy link
Member

@skjnldsv Is that fine for you?

Nice catch! Seems like it was forgotten!
Let's go for the min-width: 0; 👍

@skjnldsv skjnldsv added 1. to develop Accepted and waiting to be taken care of 28-feedback 30-feedback and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jul 30, 2024
@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants