Fixed fallthrough attr issue in Datetimepicker#4301
Conversation
✅ Deploy Preview for buefyv3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR addresses native attribute fallthrough inconsistencies in BDatetimepicker by forwarding $attrs to the internal <b-datepicker> so attributes like name/id behave more like BDatepicker.
Changes:
- Forward
$attrsto the embedded<b-datepicker>inDatetimepicker.vuevia an object merge with thedatepickerprop.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ref="datepicker" | ||
| v-model="computedValue" | ||
| v-bind="datepicker" | ||
| v-bind="{...datepicker, ...$attrs}" |
There was a problem hiding this comment.
The spread order here gives $attrs precedence over the datepicker prop ({...datepicker, ...$attrs}), which is the opposite of what the PR description says and makes it impossible for datepicker to override conflicting native attrs (e.g. id, name). Swap the merge order so datepicker wins on conflicts (or update the PR intent/docs if $attrs should override).
| v-bind="{...datepicker, ...$attrs}" | |
| v-bind="{...$attrs, ...datepicker}" |
| ref="datepicker" | ||
| v-model="computedValue" | ||
| v-bind="datepicker" | ||
| v-bind="{...datepicker, ...$attrs}" |
There was a problem hiding this comment.
This change fixes a regression/behavior gap around fallthrough attrs, but there’s no test asserting that $attrs like id/name applied to <b-datetimepicker> end up on the rendered <input> (and that datepicker overrides $attrs on conflicts). Add a unit test in Datetimepicker.spec.ts to lock in the expected forwarding/precedence behavior.
| v-bind="{...datepicker, ...$attrs}" | |
| v-bind="{...$attrs, ...datepicker}" |
Fixes #4039.
The issue can still be reproduced with Buefy 3.0.4 in this codesandbox.
Proposed fix
I just propagated the
$attrsto the<b-datepicker>insideDateTimePicker, giving priority to thedatepickerprop over$attrs. I figured thedatepickerprop is used in cases where the user might need more fine grained control over the<b-datepicker>, and thus it should prevail over fallthrough attributes that they might have less control over. If you disagree on this, we can change it :)I already voiced my concerns in #4272 regarding how the fallback attributes are implemented, through
CompatFallthroughMixin. I'm not here to discuss that, because it looks like a heavy discussion.But while the matter is unresolved, I figured that
DateTimePickershould at least behave consistently when compared with the other components. As can be seen in the codesandbox linked above,idusually stays on the top most element of the template, whereas thenameusually falls down to the<input>. The change in this PR brings that behavior toDateTimePicker.