Skip to content

Implement Unsaved Indicator#95

Merged
joostdevries merged 16 commits intoember-cli:masterfrom
Gaurav0:unsaved
Aug 3, 2015
Merged

Implement Unsaved Indicator#95
joostdevries merged 16 commits intoember-cli:masterfrom
Gaurav0:unsaved

Conversation

@Gaurav0
Copy link
Copy Markdown
Contributor

@Gaurav0 Gaurav0 commented Jul 28, 2015

No description provided.

Comment thread app/gist/controller.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems like this could just be a computed property

@joostdevries
Copy link
Copy Markdown
Member

Wouldn't something like https://github.com/lolmaus/ember-cli-stained-by-children solve this?

@Gaurav0
Copy link
Copy Markdown
Contributor Author

Gaurav0 commented Jul 29, 2015

@stefanpenner @rwjblue I must be missing something obvious. I can't figure out how to make unsaved a computed property.

@Gaurav0
Copy link
Copy Markdown
Contributor Author

Gaurav0 commented Aug 1, 2015

@rwjblue @stefanpenner @joostdevries Refactored. Please review again.

Comment thread app/components/run-or-live-reload.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we use an action from the template instead of this observer?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope. Not until Ember 2.0. emberjs/ember.js#5433

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

doesn't this work:

<input type="checkbox" value={{isChecked}} onclick={{action "foo" value="target.checked"}} />

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Found a solution. We can write our own checkbox component.

@joostdevries
Copy link
Copy Markdown
Member

there are no observers ;-)

@Gaurav0
Copy link
Copy Markdown
Contributor Author

Gaurav0 commented Aug 2, 2015

@joostdevries I wish. I unraveled the observer everyone complained about and ended up with 3.

@Gaurav0
Copy link
Copy Markdown
Contributor Author

Gaurav0 commented Aug 2, 2015

@stefanpenner @rwjblue Still waiting for a hint as to how all this could have been done with a computed property.

@stefanpenner
Copy link
Copy Markdown
Contributor

I'm confused why @joostdevries example doesn't work for this case

@Gaurav0
Copy link
Copy Markdown
Contributor Author

Gaurav0 commented Aug 2, 2015

@stefanpenner I really don't know. I just tried it and it didn't work.

@Gaurav0
Copy link
Copy Markdown
Contributor Author

Gaurav0 commented Aug 3, 2015

@joostdevries @stefanpenner @rwjblue Observers removed. Contortions added. Please review again.

Comment thread app/gist/controller.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not just send contentsChanged here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok. will do.

@Gaurav0
Copy link
Copy Markdown
Contributor Author

Gaurav0 commented Aug 3, 2015

@joostdevries Updated.

@joostdevries
Copy link
Copy Markdown
Member

👍

joostdevries pushed a commit that referenced this pull request Aug 3, 2015
Implement Unsaved Indicator
@joostdevries joostdevries merged commit a49183a into ember-cli:master Aug 3, 2015
@Gaurav0 Gaurav0 deleted the unsaved branch March 21, 2016 13:49
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.

4 participants