8000 docs: migration guide for changes to show/hide related props by dwgray · Pull Request #2386 · bootstrap-vue-next/bootstrap-vue-next · GitHub
[go: up one dir, main page]

Skip to content

docs: migration guide for changes to show/hide related props #2386

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

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

dwgray
Copy link
Member
@dwgray dwgray commented Nov 26, 2024

Describe the PR

This is an attempt to encapsulate the prop changes in #1821. I will also get to the emit changes, but they were more straightforward so I'll tack them separately.

@xvaara In the process of gong through the code for #1821 I found a number of things that I either don't understand or are internally inconsistent (as well as the bug I filed yesterday #2385)

I'm going to list them here for now, but happy to pull them out as a bug or bugs if you agree they are issues:

  • Is there a reason that transProps, noAnimation, and noFade aren't implemented on BAccordianItem and BAccordian? They're commented out and it feels like the should be implemented.
  • BAlert and BCarousel both have fade rather than noFade - for carousel, this makes a lot of sense since this is referring to the transition within the component that defaults to slide, but for BAlert it seems like it might be better to have it follow the semantics of the other show/hide components. Certainly if we ever want the option to implement RFC: Unify BToast and BAlert (and maybe useShowHide Alert) #2366, we'd want to line up the semantics now.
  • toggle-prevented event is missing on BCollapse, BNavItemDropdown
  • *-prevented should have value arg of BvTriggerableEvent
  • BOffCanvas, BPopover, and BToast don't declare events toggle or toggle-prevented, but they expose toggle, so I think they will emit those events?

Small replication

N/A

PR checklist

What kind of change does this PR introduce? (check at least one)

  • Bugfix 🐛 - fix(...)
  • Feature - feat(...)
  • ARIA accessibility - fix(...)
  • Documentation update - docs(...)
  • Other (please describe)

The PR fulfills these requirements:

  • Pull request title and all commits follow the Conventional Commits convention or has an override in this pull request body This is very important, as the CHANGELOG is generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be denied

Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
pkg-pr-new bot commented Nov 26, 2024

Open in Stackblitz

npm i https://pkg.pr.new/bootstrap-vue-next/bootstrap-vue-next@2386
npm i https://pkg.pr.new/bootstrap-vue-next/bootstrap-vue-next/@bootstrap-vue-next/nuxt@2386

commit: 861da03

@VividLemon VividLemon merged commit 66c5d28 into bootstrap-vue-next:main Nov 26, 2024
4 checks passed
A4A0
@xvaara
Copy link
Contributor
xvaara commented Nov 26, 2024

I'm quite sure there are some mistakes there. The whole pr got out of hands and I didn't have the time or the energy to go through everything in it.

I'm going to list them here for now, but happy to pull them out as a bug or bugs if you agree they are issues:

  • Is there a reason that transProps, noAnimation, and noFade aren't implemented on BAccordianItem and BAccordian? They're commented out and it feels like the should be implemented.

On BAccordianItem it passes down the attrs to BCollapse. So they aren't defined as props, but work. Create an issue to keep track.

  • toggle-prevented event is missing on BCollapse, BNavItemDropdown
  • *-prevented should have value arg of BvTriggerableEvent
  • BOffCanvas, BPopover, and BToast don't declare events toggle or toggle-prevented, but they expose toggle, so I think they will emit those events?

I'll create a pull request for these. Check out if I missed anything.

@dwgray dwgray deleted the show-hide-docs branch December 9, 2024 00:18
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.

3 participants
0