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

docs(BButtonGroup): update component references #1580

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dploc96
Copy link
Contributor

@dploc96 dploc96 commented Nov 10, 2023

Describe the PR

Change on BButtonGroup & BButtonToolbar

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)

  • 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

@dploc96 dploc96 changed the title Update BButtonGroup & BButtonToolbar component references docs(BButtonGroup): update component references Nov 10, 2023
description: `When set to 'true', disables the component's functionality and places it in a disabled state`,
},
{
prop: 'event',
Copy link
Member

Choose a reason for hiding this comment

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

So, instead of declaring the extended props, say those in BLink, instead what we should do is find a way to extend the props list.

This would make it so you don't need to change like 10 other files if BLink changes (or another thing that extends another, like BProgress)

What I would like to see is in the rendered table section for the component reference, a new "group" is made, the group has the header of its extended props, then in its section it declares all of its data.

What I would like to see is something like this:
image

(I'd like to see the "main props" without a formal "group", all the main ones are just at the top, implicitly stating that they are the "main" props)

Its a bit of a challenge, technically speaking. You'd have to: create a new property on the component reference data, import the sub-data, then copy the omit() function to remove keys like omit(BButtonProps, ['variant']), put the correct prop data on the component reference, then go into the Vue file and have the logic build out. I'm also doubting that BTable is in a position to fulfill this out of the box.

However, when its done, it will be a lot more straightforward than re-declaring each prop

Copy link
Member

Choose a reason for hiding this comment

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

(#1584)

@dploc96 dploc96 marked this pull request as draft November 17, 2023 00:26
@@ -10,16 +10,19 @@ export interface ComponentReference {
type: string
description?: string
default?: unknown
children?: Omit<ComponentReference['props'][number], 'children'>[]
Copy link
Member

Choose a reason for hiding this comment

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

A better way would be to extend the entire props list at once. Rather than declaring a prop a child of something else. Then just using the entire list

Of course, you will need to omit the real values, you can probably reuse the util -- its called omit. I think its in object.ts

Something like

props: [],
parentProps: {
  BLink: omit(BLinkProps, ['event', '/* some other omitted props*/]),
  SomeOtherFakeComponent: ... // and so on
}

You can then use Object.entries and whatnot. The parentProps is probably something like Record<string, PropsReference> where you'd need to extract the stuff from "props" into its own type

Copy link
Contributor

github-actions bot commented Jan 5, 2024

This PR is stale because it has been open for 45 days with no activity. It will not be auto-closed

@github-actions github-actions bot added the stale There has been no additional replies or questions and the thread is assumed closed label Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale There has been no additional replies or questions and the thread is assumed closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants