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

TF-toasts component implementation #2

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

Conversation

melissaroman
Copy link
Collaborator

@spencer516 @ksin @BrianSipple

  • Initial implementation for the tf-toasts
  • Added some super basic CSS (only for success) to see it better but that can easily be ripped out once we have our CSS stuff complete
  • Obviously questions, comments, etc. are great!
  • Didn't add animations in since we had talked about pushing that back until later.

README.md Outdated
**Template:**
```hbs
{{!-- Inline form --}}
{{#tf-toast type='positive' message="Positive alert message." onClick=(action "myAction")}}
Copy link

Choose a reason for hiding this comment

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

No # in inline form.

README.md Outdated

## Building
<div class="c-toast u-toast-neutral">Neutral alert message.</div>
Copy link

Choose a reason for hiding this comment

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

BEM syntax would be tf-toast and tf-toast--neutral

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @ksin! Will update.

@@ -1,3 +1,2 @@
<h2 id="title">Welcome to Ember</h2>

{{#tf-toaster}}{{/tf-toaster}}
Copy link

Choose a reason for hiding this comment

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

Since you're not yielding anything, it's probably fine to just make this inline {{tf-toaster}}

@ksin
Copy link

ksin commented Dec 30, 2016

I think some more documentation and tests for the components and services will help. It's a little tricky to reason about how this is used right now. Correct me if I'm wrong:

The consumer is expected to place tf-toaster in the root template. Toasts are displayed/dismissed by calling methods in the tf-toast-service which is injected into the invoking controller/component.

I think this is viable way of implementing toasts. I like that you don't need to keep any state in the invoking controller/component and it's all action driven. I would not expose the messages from the service. For example, what you're doing here. I would just implement a popMessage method on the service that does the same thing. A clearMessages method would be useful as well.

I think @spencer516 had a different idea of how to implement toasts using ember-wormhole, so I'm curious what others think about this approach in comparison.

@melissaroman
Copy link
Collaborator Author

@ksin Totally. I needed to add documentation and tests (started with some docs...) but then wanted to get feedback on this implementation first. And yes, you're right in how it's being used.

We can chat about other ideas / ember-wormhole next week too.

@ksin
Copy link

ksin commented Dec 30, 2016

Let's also have a discussion as a group on what the expected data flow is for saving models / validations because that should directly effect how we implement this.

@melissaroman melissaroman force-pushed the component-implementation branch from 80fe9a5 to f450a34 Compare February 8, 2017 00:05
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