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(files): fixed breadcrumbs dissapearing on narrow screens #44162

Merged
merged 2 commits into from
Mar 15, 2024

Conversation

emoral435
Copy link
Contributor

@emoral435 emoral435 commented Mar 12, 2024

Summary

NcBreadcrumbs already handled for when breadcrumbs are too long for the view-width by making the unmaintainable breadcrumbs composed into a drop-down list, so we only have to care about hiding the breadcrumbs on small view ports. I tried 420, as that is what we use everywhere for standard, but 400 worked much better with the styling of the navbar.

Screenshot

firefox_oxj23hr7fI.mp4

Checklist

@emoral435 emoral435 requested review from susnux, arublov and szaimen March 12, 2024 15:12
@emoral435 emoral435 self-assigned this Mar 12, 2024
@emoral435 emoral435 requested a review from skjnldsv as a code owner March 12, 2024 15:12
@emoral435
Copy link
Contributor Author

/backport to stable28

@emoral435 emoral435 changed the title Fix/breadcrumb narrow width being hidden fix(files): fixed breadcrumbs dissapearing on narrow screens @emoral435 Mar 12, 2024
@susnux susnux changed the title fix(files): fixed breadcrumbs dissapearing on narrow screens @emoral435 fix(files): fixed breadcrumbs dissapearing on narrow screens Mar 12, 2024
@emoral435 emoral435 force-pushed the fix/breadcrumb-narrow-width-being-hidden branch from c941f99 to b7d35b1 Compare March 14, 2024 13:25
@emoral435 emoral435 enabled auto-merge March 14, 2024 13:25
Signed-off-by: Eduardo Morales <emoral435@gmail.com>
Signed-off-by: Eduardo Morales <emoral435@gmail.com>
@emoral435 emoral435 force-pushed the fix/breadcrumb-narrow-width-being-hidden branch from b7d35b1 to 5f29c02 Compare March 15, 2024 13:39
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 15, 2024
@skjnldsv skjnldsv disabled auto-merge March 15, 2024 15:23
@skjnldsv skjnldsv merged commit c16d42f into master Mar 15, 2024
102 of 104 checks passed
@skjnldsv skjnldsv deleted the fix/breadcrumb-narrow-width-being-hidden branch March 15, 2024 15:23
@Altahrim Altahrim mentioned this pull request Mar 18, 2024
@skjnldsv
Copy link
Member

skjnldsv commented Mar 23, 2024

@emoral435 now, even on wide screens, we hide the breadcrumbs if any upload is ongoing, this seems like a regression?

My suggestion

		// Hide breadcrumbs if an upload is ongoing
		shouldShowBreadcrumbs(): boolean {
			// If we're uploading files, only show the breadcrumbs
			// if the files list is greater than 768px wide
			if (this.isUploadInProgress) {
				return this.filesListWidth > 768
			}
			// If we're not uploading, we have enough space from 400px
			return this.filesListWidth > 400
		},

cc @szaimen too
Because right now, we cannot browse files while an upload is ongoing, I think this is an issue :)

@emoral435
Copy link
Contributor Author

@skjnldsv Agreed 👍 Will make a PR to fix this. However, I will test to see if the last return condition is even needed, as within somewhere in FilesList there already is a feature to truncate the breadcrumbs around that view port width anyways, so having two places hide the breadcrumbs just increases the complexity / amount of things we would have to find and then change in the future in case we decide to change this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opening sidebar hides broadscrombs totally with some screen resolutions
3 participants