Skip to content
This repository has been archived by the owner on Jul 12, 2020. It is now read-only.

Fix cropped nested expandable content in panels #137

Merged
merged 1 commit into from
Mar 30, 2020

Conversation

ang-zeyu
Copy link

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] Bug fix

Fixes MarkBind/markbind#1166

What changes did you make? (Give an overview)
Reset the max-height of the panel content wrapper to its appropriate value
after expanding the panel, so that it can react to changes in the height of its content.

Provide some example code that this change will affect:

Resetting of maxHeight to appropriate values:
( it needs to be reset before collapsing as well so that the collapse animation would play )

afterExpand(el) {
  el.style.maxHeight = 'max-content';
},
beforeCollapse(el) {
  el.style.maxHeight = `${el.scrollHeight}px`;
},

Is there anything you'd like reviewers to focus on?
na

Testing instructions:

  • nested panels' content should no longer be 'cropped out' of main panel

Proposed commit message: (wrap lines at 72 characters)

Fix cropped nested expandable content in panels

After finishing the panel expand animation or before the collapse
animation, the max height of the panel content wrapper is not
reset to their appropriate values.
This causes expandable content inside panels, such as nested panels.

Let’s do so, allowing the outer panel to react to changes in its
content height after expanding it.

After finishing the panel expand animation or before the collapse
animation, the max height of the panel content wrapper is not
reset to their appropriate values.
This causes expandable content inside panels, such as nested panels.

Let’s do so, allowing the outer panel to react to changes in its
content height after expanding it.

/*
Otherwise, since the vue transition is dependent on localExpanded, we have to manually
set our own transition end handlers here for the initial loading of the content.
Copy link
Author

Choose a reason for hiding this comment

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

Context:
vue uses v-show/v-if to detect transition changes, and there is no way to explicitly trigger the vue animation process.
Hence, the additions here manually trigger the new afterExpand hook, the only difference being that the event listener is removed once done.

For the above however ( this.expandedBool ), we want to loading to feel 'instant' ( even though it isn't ), hence there's no animation there

@ang-zeyu ang-zeyu requested a review from nbriannl March 30, 2020 12:56
Copy link

@marvinchin marvinchin left a comment

Choose a reason for hiding this comment

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

Haven't had the chance to test this out functionally, but the code looks good to me.

@marvinchin
Copy link

@ang-zeyu just wondering - would the height be computed for content that isn't immediately fully loaded?

@ang-zeyu
Copy link
Author

Thanks for getting to this so quickly!

would the height be computed for content that isn't immediately fully loaded?

Yup, the only change here essentially is that after the end of expanding a panel, the max-height is reset to max-content to allow the container to expand if there's more expandable content inside it ( panels ).

This then necessitates also 'resetting' the max-height to the scrollHeight of the content before collapsing the panel again.

@marvinchin
Copy link

Cool, lets get this released then 🙂

@marvinchin marvinchin added this to the v2.0.1-markbind.39 milestone Mar 30, 2020
@marvinchin marvinchin merged commit 400d198 into MarkBind:master Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants