Skip to content

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 arublov, susnux 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
@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
@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

4 participants