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

Persistent State #164

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

Conversation

widmoser
Copy link
Contributor

@widmoser widmoser commented Dec 4, 2015

This pull requests adds the functionality of storing current container sizes in the local storage.

Fixes #13.

I've added some tests. I can add more if needed. Also I can update the README to reflect the changes if desired.

The basic idea is to assign a unique reconstructable id to every layout container and save the size as uncollapsedSize whenever a split bar is moved.

…r_components

This helps in case the repository is cloned inside a larger project that has a different bower setup (e.g. for angular when the bower_components is inside the app folder)
This is achieved by adding an optional attribute layout-id in the markup to uniquely identify a ui-layout. The children are identified by their indices which are now correctly updated whenever they change.
@widmoser
Copy link
Contributor Author

widmoser commented Dec 4, 2015

Note that I intentionally omitted saving the collapsed state, because I need to read the code in more detail to understand it fully. Maybe the collapsing implementation can be simplified, before extending this feature to include it.

@petrsimon
Copy link
Contributor

I'm very interested in this, but there's so much formatting changes... you think you could clean that up a bit and push only the implementation details? Cheers ;)

@SomeKittens
Copy link
Contributor

additionally, you've added a .bowerrc which is outside the scope of this PR.

@widmoser
Copy link
Contributor Author

widmoser commented Dec 9, 2015

Ok I will try to clean everything up.

@widmoser
Copy link
Contributor Author

widmoser commented Dec 9, 2015

I cleaned up the diff. I would really appreciate though, if we could agree on a consistent formatting style (space before if, space after function and so on), because it would make life for developers that are using automated formatters easier. Maybe this could be done in a separate PR. What do you think @petrsimon, @SomeKittens?

@widmoser
Copy link
Contributor Author

widmoser commented Jan 5, 2016

Did you have a chance to check it @SomeKittens, @petrsimon? Should we merge it?

@petrsimon
Copy link
Contributor

I'm really sorry guys, I've been completely swamped lately. I will start working on ui-layout again in a month or so. But I will try to find time and look at this sooner unless it's dealt with even sooner :)

@@ -83,7 +86,7 @@ angular.module('ui.layout', [])
if(!beforeContainer.collapsed && !afterContainer.collapsed) {
// calculate container positons
var difference = ctrl.movingSplitbar[position] - lastPos;
var newPosition = ctrl.movingSplitbar[position] - difference;
var newPosition = ctrl.movingSplitbar[position] - difference; // TODO: this computation is unnecessary, newPosition === lastPos
Copy link
Contributor

Choose a reason for hiding this comment

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

Open an issue for this instead of // TODO

@SomeKittens
Copy link
Contributor

Did another review pass, had a few questions.

@widmoser
Copy link
Contributor Author

Thanks for the review @SomeKittens. Sorry for the delay, I've just seen the comments. I will have a look at it as soon as I have some time.

@SomeKittens
Copy link
Contributor

Let me know when this has been cleaned up

@widmoser
Copy link
Contributor Author

@SomeKittens: I have included your suggestions

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