-
-
Notifications
You must be signed in to change notification settings - Fork 164
feat(BToast)! add noProgress prop, make progress show as default if m… #2695
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
Conversation
…odelValue is number. fix(useToastController): if using the deprecated show method the countdown didn't start.
|
""" WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant useToastController
participant BToastOrchestrator
participant BToast
User->>useToastController: create(toastProps)
useToastController->>BToastOrchestrator: create(toastProps)
BToastOrchestrator->>BToast: Render toast with props (including noProgress)
User->>useToastController: .destroy() (on returned control object)
useToastController->>BToastOrchestrator: Hide toast
BToastOrchestrator->>BToast: Trigger hide event
Assessment against linked issues
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a new noProgress property to allow hiding the progress bar in BToast components and updates the toast creation API to use a new create method with an optional options parameter. Key changes include:
- Updating the create API by adding an optional ToastOrchestratorCreateOptions parameter.
- Adding the noProgress prop to BToast components and adjusting the conditional rendering of the progress bar.
- Updating documentation and examples to replace the deprecated show method with create.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/bootstrap-vue-next/src/utils/keys.ts | Updated create signature to accept an optional options parameter. |
packages/bootstrap-vue-next/src/types/ComponentProps.ts | Added noProgress prop to BToastProps. |
packages/bootstrap-vue-next/src/types/ComponentEmits.ts | Adjusted event definitions by adding cancel/ok events in showHideEmits and removing them from BModalEmits. |
packages/bootstrap-vue-next/src/plugins/toastController/index.ts | Modified deprecated show method to return create(obj) instead of create(obj).show(). |
packages/bootstrap-vue-next/src/components/BToast/BToastOrchestrator.vue | Updated event handling logic for consistent resolution of the ok property. |
packages/bootstrap-vue-next/src/components/BToast/BToast.vue | Changed condition for rendering the progress bar and added the noProgress default prop. |
apps/docs/src/docs/composables/useToastController.md | Replaced deprecated show method references with create across usage examples. |
apps/docs/src/data/components/toast.data.ts | Documented the new noProgress prop in the component data. |
Comments suppressed due to low confidence (2)
packages/bootstrap-vue-next/src/utils/keys.ts:226
- Consider adding JSDoc comments to document the new 'options' parameter in the create method, outlining its purpose and expected properties.
create: (obj: ToastOrchestratorCreateParam, options?: ToastOrchestratorCreateOptions) => PromiseWithToast
packages/bootstrap-vue-next/src/components/BToast/BToast.vue:89
- Verify that the new condition correctly handles cases where progressProps might be undefined; ensure the intended behavior is maintained when both noProgress is false and no progressProps are provided.
v-if="typeof modelValue === 'number' && !props.noProgress"
packages/bootstrap-vue-next/src/components/BToast/BToastOrchestrator.vue
Show resolved
Hide resolved
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/docs/src/docs/composables/useToastController.md (2)
313-314
: Fix formatting in code exampleThere are trailing spaces in the code that should be removed to maintain consistent formatting.
- default: ({hide}) => - [ - h('h2', {class: 'text-center mb-3'}, 'Ready?'), - h('div', {class: 'd-flex justify-content-center gap-2'}, [ - h(BButton, {onClick: () => hide('ok'), size: 'lg'}, () => 'Yes'), - h(BButton, {onClick: () => hide('cancel'), size: 'lg'}, () => 'No') + default: ({hide}) => [ + h('h2', {class: 'text-center mb-3'}, 'Ready?'), + h('div', {class: 'd-flex justify-content-center gap-2'}, [ + h(BButton, {onClick: () => hide('ok'), size: 'lg'}, () => 'Yes'), + h(BButton, {onClick: () => hide('cancel'), size: 'lg'}, () => 'No')Also applies to: 316-318
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
313-313: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
314-314: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
303-303
: Remove extra blank lineThere are multiple consecutive blank lines here which should be reduced to a single blank line.
- - +🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
303-303: Multiple consecutive blank lines
Expected: 1; Actual: 2(MD012, no-multiple-blanks)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/docs/src/data/components/toast.data.ts
(1 hunks)apps/docs/src/docs/composables/useToastController.md
(10 hunks)packages/bootstrap-vue-next/src/components/BToast/BToast.vue
(2 hunks)packages/bootstrap-vue-next/src/components/BToast/BToastOrchestrator.vue
(2 hunks)packages/bootstrap-vue-next/src/plugins/toastController/index.ts
(1 hunks)packages/bootstrap-vue-next/src/types/ComponentEmits.ts
(1 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts
(1 hunks)packages/bootstrap-vue-next/src/utils/keys.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/bootstrap-vue-next/src/types/ComponentEmits.ts (1)
packages/bootstrap-vue-next/src/utils/classes.ts (1)
BvTriggerableEvent
(68-90)
🪛 LanguageTool
apps/docs/src/docs/composables/useToastController.md
[style] ~152-~152: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...t Hiding a Toast
programmatically is very simple. create
return an object that has fun...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.17.2)
apps/docs/src/docs/composables/useToastController.md
303-303: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
313-313: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
314-314: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
316-316: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (18)
packages/bootstrap-vue-next/src/types/ComponentProps.ts (1)
1252-1252
: New property added to control progress bar visibility.The addition of the
noProgress
property toBToastProps
provides a clean way to control the visibility of the progress bar in toast components. This aligns with the PR objective to enhance toast functionality.apps/docs/src/data/components/toast.data.ts (1)
85-89
: Documentation added for new noProgress property.The documentation properly describes the new
noProgress
property, its type, default value, and purpose. This ensures developers can understand and use the new feature correctly.packages/bootstrap-vue-next/src/components/BToast/BToastOrchestrator.vue (2)
31-31
: Enhanced event handling for toast actions.The logic for the
@hide
event handler has been improved to better differentiate between different types of toast dismissals. This provides more detailed feedback through thee.ok
property with three possible states (true, false, null) based on the trigger type.
45-45
: Consistent event handling for hidden event.The same improved logic has been correctly applied to the
@hidden
event handler, maintaining consistency between both event handlers and ensuring proper state propagation throughout the toast lifecycle.packages/bootstrap-vue-next/src/components/BToast/BToast.vue (3)
89-89
: Improved progress bar visibility conditionThe condition for showing the progress bar has been updated to use the new
noProgress
prop instead of relying on the presence ofprogressProps
. This is a cleaner approach that makes the intent more explicit.
90-95
: Good use of optional chaining for progressPropsUsing optional chaining (
?.
) to access properties ofprogressProps
is a safer approach that prevents errors whenprogressProps
is undefined.
138-138
: Added new noProgress propThis new prop allows explicit control over the progress bar display, aligning with the PR objective to make progress indication more intuitive.
packages/bootstrap-vue-next/src/types/ComponentEmits.ts (1)
12-13
: Consolidated 'cancel' and 'ok' events to shared interfaceMoving these event signatures to the
showHideEmits
interface improves code organization and ensures consistent event typing across components.packages/bootstrap-vue-next/src/utils/keys.ts (2)
29-29
: Added ToastOrchestratorCreateOptions importThis import is necessary for the updated
create
method signature.
226-229
: Extended create method signature with optional options parameterThe
create
method now accepts an additional optional parameter for creation options, providing more flexibility and control over toast creation. This enhancement supports the fix for the countdown timer issue mentioned in the PR objectives.apps/docs/src/docs/composables/useToastController.md (8)
39-39
: Updated basic example to use create instead of showThe documentation has been updated to use the new
create
method instead of the deprecatedshow
method in the basic example.Also applies to: 44-44, 48-48
77-77
: Updated reactivity example to use create methodThe reactive example now correctly uses the
create
method, aligning with the API changes.Also applies to: 89-90
120-120
: Updated advanced usage example to use create methodThe advanced usage example has been updated to use
create
instead ofshow
, maintaining consistency with the API changes.Also applies to: 133-134
152-153
: Updated programmatic hiding descriptionThe documentation now correctly explains that
create
returns an object with control functions includingdestroy
.🧰 Tools
🪛 LanguageTool
[style] ~152-~152: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...t Hiding aToast
programmatically is very simple.create
return an object that has fun...(EN_WEAK_ADJECTIVE)
176-192
: Improved toast destruction exampleThe example for programmatically hiding a toast has been updated to store the return value of
create
and calldestroy
on it, which is more intuitive than using a separateremove
function.
200-247
: Added new section on using promises with toastsThis new section demonstrates how to use promises with the
create
method, showing a common pattern for creating interactive toasts. This enhances the documentation by covering more advanced use cases.
259-261
: Updated script setup example to use create and destroyThe script setup example has been updated to use the new API pattern with
create
and its returneddestroy
method.Also applies to: 265-271
304-326
: Added promise toast example implementationThe implementation of the promise toast example correctly demonstrates how to use the resolveOnHide option and handle the result in a then chain.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
313-313: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
314-314: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
316-316: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
Hi there and many thanks for your efforts! I have just tried to put this merge request to practice, but it fails me. Here's a rundown of my setup and what I have tried in the form of a minimal example: Install the merge request
vite configI am not using nuxt, so I'm not including any composables separately, as per the docs: // nuxt.config.js/ts
export default defineNuxtConfig({
modules: ['@bootstrap-vue-next/nuxt'],
bootstrapVueNext: {
composables: true, // Will include all composables
// composables: {useBreadcrumb: true, useColorMode: true, all: false}, // Will include only useBreadcrumb & useColorMode
// composables: {useBreadcrumb: false, useColorMode: false, all: true} // Will include everything except useBreadcrumb & useColorMode
directives: {all: true}, // Will include all directives
css: true, // Will include the module's CSS. If set to false, you can add the CSS manually in the 'css' property below
},
css: [
// 'bootstrap/dist/css/bootstrap.min.css' // Not necessary if `css: true`
],
}) instead, this is my import { fileURLToPath, URL } from 'node:url'
import { defineConfig } from 'vite'
import vue from '@vitejs/plugin-vue'
import vueDevTools from 'vite-plugin-vue-devtools'
import Components from 'unplugin-vue-components/vite'
import { BootstrapVueNextResolver } from 'bootstrap-vue-next'
import AutoImport from 'unplugin-auto-import/vite'
// https://vite.dev/config/
export default defineConfig({
build: {
minify: false,
},
css: {
devSourcemap: true,
},
plugins: [
vue(),
Components({
resolvers: [BootstrapVueNextResolver()],
}),
AutoImport({
include: [/\.[tj]sx?$/, /\.vue$/, /\.vue\?vue/, /\.vue\.[tj]sx?\?vue/, /\.md$/,],
imports: [
'vue',
{ 'mande': ['mande'], },
],
dts: true, //'./auto-imports.d.ts',
viteOptimizeDeps: true,
eslintrc: {
enabled: true,
},
}),
vueDevTools(),
],
resolve: {
alias: {
'@': fileURLToPath(new URL('./src', import.meta.url))
},
},
}) Usage within the componentIn my component, I import the <script setup lang="ts">
import { useToastController } from 'bootstrap-vue-next'
const { create } = useToastController()
const sayHello = () => {
create({ title: 'Hello', body: 'World' })
}
</script>
<template>
<BButton size="sm" @click="sayHello(row)">Draft</BButton>
</template> Setup
|
@xdbr did you install our plugin in the main.js (or .ts) file? https://bootstrap-vue-next.github.io/bootstrap-vue-next/docs.html#installation-vue-js Also vite config is different from nuxt config. |
…lass prop since class is inherited to the same place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
apps/docs/src/data/components/popover.data.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
apps/docs/src/data/components/popover.data.ts (1)
113-116
: Added newtitleClass
prop for styling popover titles.Good addition! This new prop provides better customization by allowing separate styling for the popover's title section, which complements the
bodyClass
prop for the body section. This enhances the component's flexibility and follows the pattern of other Bootstrap Vue components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Thanks!
…odelValue is number. fix(useToastController): if using the deprecated show method the countdown didn't start.
closes #2687
Describe the PR
Breaking Change:
BPopover
andBTooltip
propcustom-class
renamed tobody-class
and addedtitle-class
Small replication
A small replication or video walkthrough can help demonstrate the changes made. This is optional, but can help observe the intended changes. A mentioned issue that contains a replication also works.
PR checklist
What kind of change does this PR introduce? (check at least one)
fix(...)
feat(...)
fix(...)
docs(...)
The PR fulfills these requirements:
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 deniedSummary by CodeRabbit