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(ui5-bar): provide responsive paddings support #9820

Merged
merged 10 commits into from
Sep 27, 2024

Conversation

unazko
Copy link
Contributor

@unazko unazko commented Sep 5, 2024

  • Responsive paddings concept is now applied to ui5-bar component display.

Fixes: #7359

@unazko unazko self-assigned this Sep 5, 2024
@unazko unazko requested a review from didip1000 September 9, 2024 12:03
Copy link
Contributor

@didip1000 didip1000 left a comment

Choose a reason for hiding this comment

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

The paddings don't look right in full screen, they're are all on the right.

@unazko
Copy link
Contributor Author

unazko commented Sep 10, 2024

The paddings don't look right in full screen, they're are all on the right.

The left padding is fixed on large screens.

@unazko unazko requested a review from didip1000 September 10, 2024 16:32
Copy link
Contributor

@didip1000 didip1000 left a comment

Choose a reason for hiding this comment

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

Works on Chrome, Firefox, and Safari (mobile as well), but can you change the sizes to be in rem instead of px to be consistent?

@unazko
Copy link
Contributor Author

unazko commented Sep 12, 2024

Works on Chrome, Firefox, and Safari (mobile as well), but can you change the sizes to be in rem instead of px to be consistent?

The S,M, L, XL brakepoints are represented in pixels in the documentation. I suspect that it will be less readable if we change to rem in this case. Please checkout the similar container query implementation in the ui5-page component.

@unazko unazko requested a review from didip1000 September 12, 2024 04:42
Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

All the CSS is copy/pasted 3 times, please extract to variables and only change the variables according to media size, for example:

.ui5-bar-startcontent-container {
	padding-inline-start: var(--_ui5_bar-start-container-padding-start);
}
.ui5-bar-endcontent-container {
	padding-inline-end: var(--_ui5_bar-end-container-padding-end);
}
.ui5-bar-root .ui5-bar-content-container {
	min-width: calc(30% - calc(var(--_ui5_bar-start-container-padding-start)
	+ var(--_ui5_bar-end-container-padding-end)
	+ (2*var(--_ui5_bar-mid-container-padding-start-end))));
}

@container (width < 600px) {
	* {
		--_ui5_bar-start-container-padding-start: var(--_ui5_bar-start-container-padding-start_S);
		--_ui5_bar-end-container-padding-end: var(--_ui5_bar-end-container-padding-end_S)
	}
}

@container (width > 599px) and (width < 1440px) {
	* {
		--_ui5_bar-start-container-padding-start: var(--_ui5_bar-start-container-padding-start_M_L);
		--_ui5_bar-end-container-padding-end: var(--_ui5_bar-end-container-padding-end_M_L);
	}
}

@container (width > 1439px) {
	* {
		--_ui5_bar-start-container-padding-start: var(--_ui5_bar-start-container-padding-start_XL);
		--_ui5_bar-end-container-padding-end: var(--_ui5_bar-end-container-padding-end_XL);
	}
}

@unazko unazko requested a review from vladitasev September 27, 2024 08:39
@unazko unazko merged commit 7d9cb85 into SAP:main Sep 27, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: ui5-bar - implement responsive paddings
3 participants