Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

fix: nesting and height issues #772

Merged
merged 1 commit into from
Jan 17, 2020
Merged

Conversation

asangma
Copy link
Contributor

@asangma asangma commented Jan 17, 2020

Related Issue: #771

Summary

Don't call it a comeback...but the 100% heights are making a comeback.
When in non-detached mode, the flow-item footers were not sticking to the bottom.

Outside components being but into flow-item will still need to set their own heights (i.e. to 100%).

Would be nice to get this into a patch maybe?

@asangma asangma added this to the Grob Gob Glob Grod milestone Jan 17, 2020
@asangma asangma requested a review from driskull January 17, 2020 17:38
@asangma asangma requested a review from a team as a code owner January 17, 2020 17:38
@asangma asangma self-assigned this Jan 17, 2020
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

@asangma asangma changed the title fix(shell-panel, flow, flow-item): nesting and height issues fix: nesting and height issues Jan 17, 2020
@asangma
Copy link
Contributor Author

asangma commented Jan 17, 2020

Thanks @driskull!

@asangma asangma merged commit 2111987 into master Jan 17, 2020
@asangma asangma deleted the asangma/panel-height-issues branch January 17, 2020 18:53
@pr3tori4n
Copy link
Contributor

I don't think this is the right way to go. Making Shell > Flow > Block height 100% seems very specific to the mapmaker. This is a brittle approach that makes significant assumptions about the integration pattern.

Outside of this one use case, I see these heights either being inert (due to not being inside a container that has height: 100% , or causing problems by being inside a container with a defined height when the desire is not for the component to stretch.

@asangma
Copy link
Contributor Author

asangma commented Jan 17, 2020

seems very specific to the mapmaker

This is specific to full page data viz apps...the main aim of these components.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants