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(offcanvas): add static backdrop option #4367

Merged

Conversation

jnizet
Copy link
Member

@jnizet jnizet commented Jul 29, 2022

this aligns with Bootstrap 5.2.0 functionality

Before submitting a pull request, please make sure you have at least performed the following:

  • read and followed the CONTRIBUTING.md and DEVELOPER.md guide.
  • built and tested the changes locally.
  • added/updated any applicable tests.
  • added/updated any applicable API documentation.
  • added/updated any applicable demos.

this aligns with Bootstrap 5.2.0 functionality
@maxokorokov maxokorokov added this to the 13.1.0 milestone Aug 1, 2022
Copy link
Member

@maxokorokov maxokorokov left a comment

Choose a reason for hiding this comment

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

Hey, @jnizet!
LGTM apart from small API typing issue in the NgbOffcanvasConfig

*
* Default value is `true`.
*/
backdrop?: boolean;
backdrop?: boolean | 'static';
Copy link
Member

Choose a reason for hiding this comment

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

You should type backdrop: boolean | 'static' = true in NgbOffcanvasConfig too (below in this file).
At the moment it is backdrop = true and typescript is unhappy when one tries to set it to 'static':

Screenshot 2022-08-02 at 16 01 26

@jnizet
Copy link
Member Author

jnizet commented Aug 2, 2022

Good catch again @maxokorokov. Fixed.

Copy link
Member

@maxokorokov maxokorokov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@maxokorokov maxokorokov merged commit fe4c798 into ng-bootstrap:master Aug 3, 2022
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.

None yet

2 participants