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

always full height #978

Closed
wants to merge 2 commits into from
Closed

Conversation

Rogermax
Copy link
Contributor

Description

We merge this PR: #962 with a pending change (make height always 100% not with an optional prop)

@Rogermax Rogermax requested a review from a team as a code owner December 16, 2024 11:20
Copy link

github-actions bot commented Dec 16, 2024

🔍 Visual review for your branch is published 🔍

Here are the links to:

Copy link
Contributor

@nlopin nlopin left a comment

Choose a reason for hiding this comment

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

🙌

@nlopin
Copy link
Contributor

nlopin commented Dec 16, 2024

@Rogermax feel free to use the merged version with props. I noticed that it changes layout UI tests quite significantly and I want to cross-check with the team if it is what we want to have

@nlopin
Copy link
Contributor

nlopin commented Dec 16, 2024

@sauldom102 @josepjaume could you check the UI Tests for layouts?

We change the vertical space significantly, so maybe it is no-go for us. From my side, I wouldn't do the change and kept the merged solution with the fullHeight prop

@Rogermax
Copy link
Contributor Author

@Rogermax feel free to use the merged version with props. I noticed that it changes layout UI tests quite significantly and I want to cross-check with the team if it is what we want to have

Perfect!

@nlopin
Copy link
Contributor

nlopin commented Jan 8, 2025

I close this one since it breaks other layouts

@nlopin nlopin closed this Jan 8, 2025
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.

2 participants