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

drop private @class argument of <BsModal> #1755

Merged
merged 1 commit into from Feb 25, 2022

Conversation

jelhan
Copy link
Contributor

@jelhan jelhan commented Feb 21, 2022

The @class argument of <BsModal> was added a long-time ago when that component was refactored to a tag-less component. It was not documented as a public API. A consumer can use class attribute instead. I think it is time to remove it.

There are some other components as well for which we have private @class argument:

  • <BsAccordion::Item::Body>
  • <BsDropdown::Menu>
  • <BsLinkTo>
  • <BsNavbar::LinkTo>
  • <BsPopover> and <BsPopover::Element>
  • <BsTooltip> and BsTooltip::Element`

For <BsAccordion::Item::Body>, <BsDropdown::Menu>, <BsPopover>, <BsPopover::Element>, <BsTooltip> and <BsTooltip::Element> I created pull requests to drop it as well: #1756, #1758, #1759 and #1760

For <BsLinkTo> and <BsNavbar::LinkTo> we can not drop the @class argument because we need to set the class on a yielded component instance.
https://github.com/kaliber5/ember-bootstrap/blob/f2ca93d7bf3ebd50de695f1c2800f7694e9bb07d/addon/components/bs-nav.hbs#L6
https://github.com/kaliber5/ember-bootstrap/blob/f2ca93d7bf3ebd50de695f1c2800f7694e9bb07d/addon/components/bs-navbar.hbs#L8

For <BsLinkTo> we could reduce the number of cases in which we need to use @class argument. Created a pull request to use class attribute for one case, in which it was possible. #1757

@simonihmig simonihmig merged commit df88a2a into master Feb 25, 2022
@simonihmig simonihmig deleted the drop-class-argument-of-bs-modal branch February 25, 2022 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants