feat(toast): Toast implementation #3103

benouat · 2019-04-04T12:18:36Z

My final attempt at implementing Bootstrap Toast

The PR basically introduce:

<ngb-toast> component. it just render bootstrap markup, with some inputs, and one single output that emit after a given delay in ms.
NgbToastConfig to override any of the default config values used to initialise above inputs.
An overview page, explaining what are toast, how to use them, and also a full fledged how-to tutorial showing how to implement a global toast service inside any application.

DISCLAIMER: this PR only introduce a declarative way of creating a toast. Don't look around for any NgbToastService, there is none! I did not implement it on purpose. If you really think it should be done, I am just waiting, go come challenge me !! 😆

codecov-io · 2019-04-05T07:03:11Z

Codecov Report

Merging #3103 into master will increase coverage by 0.05%.
The diff coverage is 97.14%.

@@            Coverage Diff             @@
##           master    #3103      +/-   ##
==========================================
+ Coverage   92.03%   92.09%   +0.05%     
==========================================
  Files          91       94       +3     
  Lines        3052     3087      +35     
  Branches      504      508       +4     
==========================================
+ Hits         2809     2843      +34     
  Misses        179      179              
- Partials       64       65       +1

Flag	Coverage Δ
#e2e	`62.94% <48.57%> (-0.17%)`	⬇️
#unit	`89.55% <97.14%> (+0.08%)`	⬆️

Impacted Files	Coverage Δ
src/index.ts	`100% <ø> (ø)`	⬆️
src/toast/toast.module.ts	`100% <100%> (ø)`
src/toast/toast-config.ts	`100% <100%> (ø)`
src/toast/toast.ts	`96.42% <96.42%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a2d0cc...3245b70. Read the comment docs.

demo/src/app/components/toast/overview/toast-overview.component.html

fbasso

Thanks for your PR @benouat ! I let a couple of comments in my review, mainly typo fixes.

demo/src/app/components/toast/overview/toast-overview.component.html

demo/src/app/components/toast/demos/custom-header/toast-custom-header.html

demo/src/app/app.module.ts

demo/src/app/components/toast/demos/inline/toast-inline.html

src/toast/toast.spec.ts

src/toast/toast.ts

fbasso · 2019-05-23T15:33:34Z

I'm not also 100% sure that letting the widget visibility managed only by the developer is a good thing. I mean, for some use cases, it can render thing more complex than expected.

For example, what about a stateless message, as something the app wants to display a notification few seconds, and let the widget 'die' by itself.

As a developer, I would expect something like:

<ngb-toast delay="1000" autohide="true">
  I am a simple static toast.
</ngb-toast>

... and that's it !

I need more time to thing about it :)

maxokorokov

Hey @benouat!

Looks good as a starting API and we'll see if we need a service later.

Left some comments and most notably the big ones are:

we should have a demo where clicking on the cross would close the toast (simple one)
in the overview we should state somewhere early in the beginning that it's up to you to position toasts (like bootstrap does)
some tests will become invalid if you simplify autohide

Cheers,
Max

maxokorokov · 2019-05-27T09:25:22Z

package.json

@@ -81,7 +81,7 @@
    "@types/marked": "^0.6.1",
    "@types/node": "~8.9.4",
    "@types/prismjs": "^1.9.0",
-    "bootstrap": "4.0.0",
+    "bootstrap": "4.2.1",


Please don't forget to remove from this commit before merging. Same for yarn.lock changes

I am confused... this change is dependant on that. without bootstrap@4.2.1 toast won't work... I really don't like to bump this version in another commit...

I mean we have several other PRs waiting that need BS 4.2 or 4.3. We won't bump BS in all of them I guess.

Just wanted to have a separate commit/PR with the bump of dependencies. Locally when testing/reviewing it's not a problem to bump it manually.

src/toast/toast.scss

src/toast/toast.ts

src/toast/toast.scss

demo/src/app/components/toast/overview/toast-overview.component.html

src/toast/toast.spec.ts

maxokorokov · 2019-05-27T15:00:55Z

src/toast/toast.spec.ts

+
+    it('should emit hide output after default delay (500ms)', fakeAsync(() => {
+         const fixture = createTestComponent(`<ngb-toast header="header" (hide)="hide()">body</ngb-toast>`);
+         tick(500);


Maybe check that is wasn't emitted before ? Same for the next test

maxokorokov · 2019-05-27T15:02:07Z

src/toast/toast-config.ts

@@ -0,0 +1,40 @@
+import {Injectable} from '@angular/core';
+
+export type AriaLiveValue = 'polite' | 'alert';


Don't see much utility in this type personally.

it is used only in this file

it makes documentation more complex

I removed it, but I have to duplicate the type info for Typescript to be happy.
For example, I am not keen about ariaLive: 'polite' | 'alert' = 'polite'; either... but without it does not compile

The big upside for me would be the change in this:

It would just say type: 'polite' | 'alert'

benouat · 2019-05-28T09:07:32Z

@peterblazejewicz @fbasso @maxokorokov thanks to all of you for your reviews!
I have addressed all your comments, and just pushed again.

@maxokorokov @fbasso

I removed the weird part regarding autohide and close button.
I added a new demo to illustrate clicking on close. (and added a disclaimer to the first 2 demos clicking close won't do anything)
regarding the formatting stuff... I left it as it is, I really want to take 5.0-rc as an opportunity to clean up everything related to clang.
I removed the bootstrap bumping version from package.json

I'll open another PR to change slightly the big demo. I'll update the danger toast to be [autohide]="false" and only disappear when clicked on the body (to illustrate closing without using × and (hide))

Implementation of twbs/bootstrap##22980

benouat added component: toast needs: review labels Apr 4, 2019

benouat added this to the 5.0 milestone Apr 4, 2019

maxokorokov added breaking change type: feature labels Apr 4, 2019

benouat force-pushed the feat/toast branch 2 times, most recently from 918d638 to 62fe8f7 Compare April 5, 2019 06:59

benouat force-pushed the feat/toast branch 2 times, most recently from ecc1d58 to 7350230 Compare April 5, 2019 10:21

peterblazejewicz reviewed May 5, 2019

View reviewed changes

demo/src/app/components/toast/overview/toast-overview.component.html Outdated Show resolved Hide resolved

benouat force-pushed the feat/toast branch from 7350230 to f2dc8e3 Compare May 22, 2019 13:06

fbasso requested changes May 23, 2019

View reviewed changes

maxokorokov requested changes May 27, 2019

View reviewed changes

maxokorokov added needs: work and removed needs: review labels May 27, 2019

feat(toast): Toast implementation

3245b70

benouat force-pushed the feat/toast branch from f2dc8e3 to 3245b70 Compare May 28, 2019 08:59

benouat added needs: review and removed needs: work labels May 28, 2019

benouat requested a review from maxokorokov May 28, 2019 09:23

maxokorokov modified the milestones: 5.0, 5.0.0-rc.0 Jun 3, 2019

benouat added needs: merging and removed needs: review labels Jun 19, 2019

benouat merged commit bd1e9fb into ng-bootstrap:master Jun 24, 2019

mijohnsmith pushed a commit to mijohnsmith/ng-bootstrap that referenced this pull request Jun 27, 2019

feat(toast): Toast implementation (ng-bootstrap#3103)

6c35332

Implementation of twbs/bootstrap##22980

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(toast): Toast implementation #3103

feat(toast): Toast implementation #3103

benouat commented Apr 4, 2019

codecov-io commented Apr 5, 2019 •

edited

fbasso left a comment

fbasso commented May 23, 2019 •

edited

maxokorokov left a comment

maxokorokov May 27, 2019

benouat May 28, 2019

maxokorokov May 28, 2019

maxokorokov May 27, 2019

maxokorokov May 27, 2019

benouat May 28, 2019

maxokorokov May 28, 2019

benouat commented May 28, 2019 •

edited

		@@ -0,0 +1,40 @@
		import {Injectable} from '@angular/core';

		export type AriaLiveValue = 'polite' \| 'alert';

feat(toast): Toast implementation #3103

feat(toast): Toast implementation #3103

Conversation

benouat commented Apr 4, 2019

codecov-io commented Apr 5, 2019 • edited

Codecov Report

fbasso left a comment

Choose a reason for hiding this comment

fbasso commented May 23, 2019 • edited

maxokorokov left a comment

Choose a reason for hiding this comment

maxokorokov May 27, 2019

Choose a reason for hiding this comment

benouat May 28, 2019

Choose a reason for hiding this comment

maxokorokov May 28, 2019

Choose a reason for hiding this comment

maxokorokov May 27, 2019

Choose a reason for hiding this comment

maxokorokov May 27, 2019

Choose a reason for hiding this comment

benouat May 28, 2019

Choose a reason for hiding this comment

maxokorokov May 28, 2019

Choose a reason for hiding this comment

benouat commented May 28, 2019 • edited

codecov-io commented Apr 5, 2019 •

edited

fbasso commented May 23, 2019 •

edited

benouat commented May 28, 2019 •

edited