Convert RecurringSchedulePicker.jsx -> RecurringSchedulePicker.tsx
#3396
Conversation
actual-github-bot
bot
changed the title
RecurringSchedulePicker.jsx -> RecurringSchedulePicker.tsxRecurringSchedulePicker.jsx -> RecurringSchedulePicker.tsx
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller
Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
jfdoming
force-pushed
the
jfdoming/schedule-picker-types
branch
from
35b459d
to
6292e68
Compare
jfdoming
force-pushed
the
jfdoming/schedule-picker-types
branch
from
6292e68
to
2b09c53
Compare
jfdoming
changed the title
RecurringSchedulePicker.jsx -> RecurringSchedulePicker.tsxRecurringSchedulePicker.jsx -> RecurringSchedulePicker.tsx
| @@ -61,6 +63,7 @@ export function Select<const Value = string>({ | |||
| <> | |||
| <Button | |||
| ref={triggerRef} | |||
| id={id} | |||
There was a problem hiding this comment.
❓ question: why do we need an id on the buttons? And what does it do?
There was a problem hiding this comment.
It's used for the htmlFor attributes that were pre-existing in RecurringSchedulePicker. The id attribute was actually being passed as a prop but the labels weren't being correctly associated with the fields since Select didn't actually accept that prop. I suppose you can consider this an additional bugfix caught by static typing since they were supposed to be associated?
There was a problem hiding this comment.
Ah.. ok, that makes sense for input fields. But not so much for buttons I think.
There was a problem hiding this comment.
Which element should we put it on here? The htmlFor is supposed to be for an input element, but the "input" in this case is the Select.
There was a problem hiding this comment.
🤦 ignore my previous comment. I'm reviewing too many things in parallel.
--
It makes sense to put it on the Button here since the general element is Select and the button is used for opening the select popover.
| @@ -344,6 +346,7 @@ export function DateSelect({ | |||
| return ( | |||
| <View {...containerProps}> | |||
| <Input | |||
| id={id} | |||
actualbudget#3396) * Supporting types * Move `RecurringSchedulePicker.jsx` -> `RecurringSchedulePicker.tsx` * Add release notes
Relates to #1483