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

Added ability to toggle the visibility of happy-split-views #9

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Kilowhisky
Copy link

As referenced by this feature. #8

@Kilowhisky
Copy link
Author

Just found a bug with it to do with glimmer re-applying properties on re-render causing the widths of the containers to get out of whack. Working on it now.

EDIT: this might be a deeper problem since the component is modifying splitPercentage internally and it is also passed in externally. Since it breaks the actions-up data down mentality i can see why the property is being overwritten by glimmer.

@Kilowhisky
Copy link
Author

There are two way to solve this problem. I'd like your input on which way is best.

  1. The bug can be avoided if the `splitPercentage value is a property on the controller and not just a static value. That way two-way data binding takes over and the value is stored up the tree so when the attribute is re-applied it gets the appropriate value.
  2. It can also be solved by manually triggering the mouseMove event anytime one of the splitter panes is re-rendered because of an external property change. This route seems to go against the ember mindset though as it might overwrite an incoming property change.

I'd prefer #1 because it doesn't quite seem to break ember's flow. It does introduce an unexpected gotcha though.

@ZebraFlesh
Copy link
Owner

I haven't had a chance to review this yet, but based on your description it seems like the ideal solution is to leave splitPercentage as a property that gets passed in externally, but then copy its value for use internally (say, internalSplitPercentage or _splitPecentage). That way consumers can set the value as they see fit and the component would never touch it.

There's a larger unresolved issue of how components communicate with each other (especially in a parent/child relationship) that keeps getting pushed off by the ember team (and rightly so, given the impacts). See the second half of emberjs/ember.js#11170 (comment) for a deeper discussion of that issue.

@Kilowhisky
Copy link
Author

I've thought about the internal value holder for splitPercentage but in my tests i was unable to determine if a property reset was caused by the user setting the property manually or by ember deciding to re-render the component. Without that differentiation the component has no idea if it should overwrite its internal value or not.

I know the 'ember-way' of doing things would involve an action fired up anytime the splitter is resized and then a new splitPercentage being driven down. But that's needlessly complicated and breaks the addon's isolation of concerns for no real benefit.

I have been following the parentView / childView saga. Despite what they say they have indeed changed how parentView behaves and its mucked with quite a bit of things. When i was writing this PR i kept having problems where the parentView kept geting redirected to the page instead of the parent component. Very, very, very frustrating.

Kilowhisky added 4 commits July 28, 2016 10:36
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