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

feat(alert): self-closing alerts are non-dismissible by default #676

Closed
wants to merge 1 commit into from

Conversation

jnizet
Copy link
Member

@jnizet jnizet commented Sep 4, 2016

  • refactor NgbDismissibleAlert to NgbSelfClosingAlert
  • make it possible to hide the close button on self-closing alerts
  • make the self-closing alert tests a bit less convoluted

fix #660

private _popupService: PopupService<NgbAlert>;
private _timeout;

/**
* A flag indicating if the alert can be dismissed (closed) by a user. If this flag is set, a close button (in a
* form of a cross) will be displayed.
Copy link
Member

Choose a reason for hiding this comment

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

&times;? "times"? "x"? just a stupid nit here on my part, but i think of "cross" as a plus.

Copy link
Member

Choose a reason for hiding this comment

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

ok, i'm confused. how are these lines showing up as "new" when they are already present in the original file?

Copy link
Member Author

Choose a reason for hiding this comment

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

The lines appear in the source of NgbAlert (the "regular" alert component). This is a new input for NgbSelfClosingAlert, previously named NgbDismissibleAlert, which didn't have such an input: the close button was always visible in such alerts, despite the fact that they can close themselves automatically.

So I added the equivalent input here and copy-pasted the api doc from NgbAlert, to be able to show or hide the close buttons on self-closing alerts too. I also chose to make the button invisible by default, since this component is self-closing and thus doesn't need a close button most of the time.

I'll amend the PR with "an ×" instead of "a cross".

@icfantv
Copy link
Member

icfantv commented Sep 5, 2016

@jnizet just the one question and LGTM.

- refactor NgbDismissibleAlert to NgbSelfClosingAlert
- make it possible to hide the close button on self-closing alerts
- make the self-closing alert tests a bit less convoluted

fix ng-bootstrap#660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[alert] rename NgbDismissibleAlert, make it non-dismissible
2 participants