class with $attrs inheritance

[xvaara]

I think that in some components the inheritance of $attrs is little weird.

for example if I want to add a class to b-form-checkbox it goes to the input element, not to the wrapping div. on the other hand for other attrs it's great that they bind to the input.

should the class be bind to the root element and we could have inputClass and labelClass props that would bind to those. I'm having some difficulty that I have to put a wrapper element around the component and create a custom class for that to for example overwrite the padding in form-check.

[VividLemon]

That's a tough one to solve. On one hand, I think it's a remnant of what bootstrap-vue decided. Which, would mean that if we made a fixing change, that it would be a major breaking change to those components. On the other, it would be nice if there was an easy way to modify both in cases like you exampled. In such cases, a way to have non-breaking functionality would be to add optional prop attributes that only affect the wrapper. Could have wrapperClasses & wrapperAttrs and v-bind="wrapperClasses & wrapperAttrs". The $attrs would still be passed the way it is, with the added ability to modify wrapper elements.

[xvaara]

Could we just "massage" the $attrs to remove the class and put that to the root element. That would solve my problem and change as little as possible.

[VividLemon]

Not sure what you mean

[xvaara]

we could have:

const { class: rootClasses, ...attrs } = useAttrs();

and use attrs instead of $attrs in input element, and add rootClasses to computedClasses:

const computedClasses = computed(() => ({
  ...unref(getClasses(classesObject)),
  [rootClasses]: true,
}));

It works for me 😄

made an example: https://stackblitz.com/edit/vitejs-vite-a75zzj?file=src%2Fcomponents%2FBFormCheckbox2.vue,src%2FApp.vue&terminal=dev

[VividLemon]

It would work, but I still prefer the approach of having a separate prop for the two. If it was done your way, there would be no way to put a custom class onto the receiver of attrs. I think a rootAttr and rootClass prop would fill in the void and allow for both modification to the element receiving attrs, and the wrapper element.

[VividLemon]

This issue is tracked for https://github.com/orgs/bootstrap-vue-next/projects/1?pane=issue&itemId=31679414

[yanghoxom]

@VividLemon Can you share about the progress of this issue? it will support custom wrapper class for some components like BFormCheckbox, BFormRadio? for now, I want to add more class to computedClasses but can't it is quite strange when the class is inherited by input element normally, the class should be inherited by root div, we can use inputClass và and labelClass for elements inside https://github.com/bootstrap-vue-next/bootstrap-vue-next/blob/main/packages/bootstrap-vue-next/src/components/BFormCheckbox/BFormCheckbox.vue#L2C56-L2C71

[VividLemon]

@VividLemon Can you share about the progress of this issue? it will support custom wrapper class for some components like BFormCheckbox, BFormRadio? for now, I want to add more class to computedClasses but can't it is quite strange when the class is inherited by input element normally, the class should be inherited by root div, we can use inputClass và and labelClass for elements inside https://github.com/bootstrap-vue-next/bootstrap-vue-next/blob/main/packages/bootstrap-vue-next/src/components/BFormCheckbox/BFormCheckbox.vue#L2C56-L2C71

No progress. It will be one of the last things finished as there are more important things to do.

[VividLemon]

@xvaara In this commit You made them HTMLAttributes,

I didn't think about this about this before, but attrs can really be anything that falls through. This is especially true for dynamic components Where we don't know what the component will be at compile time, it could be another component.

So, it would be limiting on what attrs you're allowed to give to that component, even if that "attr" is actually becomes a prop.

It might be possible to do "HTMLAttrs | any", to give intellisense, but I don't think the TS compiler is smart enough for that ('foo' | string for instance is not allowed, 'foo' is of type string, so it breaks down, I think this would have the same core issue)

So, moving forward it should probably be "any" (maybe unknown, but some experimentation would need to happen. I'm not sure how consumers would react), similar to the internal ClassValue