TPS-1105: allow single select field to select from booleans 🚨do not merge#57
Open
mad-raccoon wants to merge 1 commit intomainfrom
Open
TPS-1105: allow single select field to select from booleans 🚨do not merge#57mad-raccoon wants to merge 1 commit intomainfrom
mad-raccoon wants to merge 1 commit intomainfrom
Conversation
WalkthroughThis PR adds changeset entries documenting fixes for boolean support in SingleSelectField, bumps the remarkable-ui dependency version, and systematically replaces logical OR operators with nullish coalescing operators across chart and form field components. Additionally, prop type signatures are expanded to accept boolean, number, and undefined values alongside strings for improved type safety. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/editors/MultiSelectFieldPro/index.tsx (1)
13-21: AlignselectedValueswith widened option types.
onChangenow supports boolean/number values, butselectedValuesis stillstring[], so callers can’t pass non-string selections andvaluesbecomes type-inconsistent. WidenselectedValuesto the same union.🛠️ Suggested fix
type MultiSelectFieldProProps = { dimension: Dimension; optionalSecondDimension?: Dimension; placeholder?: string; results: DataResponse; - selectedValues?: string[]; + selectedValues?: (string | number | boolean | undefined)[]; maxOptions?: number; setSearchValue?: (search: string) => void; onChange?: (newValues: (string | number | boolean | undefined)[]) => void; } & ChartCardHeaderProps;
🤖 Fix all issues with AI agents
In `@src/components/editors/SingleSelectFieldPro/index.tsx`:
- Line 66: Remove the debug console.log that prints selectedValue and safeValue
inside the SingleSelectFieldPro component: locate the stray
console.log('selectedValue', selectedValue, safeValue) and delete it (or replace
it with a proper non-user-data-producing debug log behind an env-level debug
flag if you need runtime diagnostics), ensuring no user data is written to
stdout/stderr.
- Around line 53-64: The safeValue lookup currently turns an already-boolean
selectedValue into undefined and doesn't handle numeric-string coercion; update
the safeValue computation (the block using selectedValueIsBoolean,
optionalSecondDimension, dimension, selectedValue, and options) to coerce
selectedValue into the option value type instead of forcing undefined: if
selectedValueIsBoolean is true, accept true/false values and also the strings
'true'/'false' (preserve booleans if already boolean); if the matched option
values are numbers, coerce numeric strings to numbers (Number or parseFloat)
before comparison; otherwise compare using strict equality to find the matching
option.value and return that value for safeValue.
In `@src/components/editors/SingleSelectFieldPro/SingleSelectFieldPro.emb.ts`:
- Around line 110-112: The meta declarations for this component are out of sync
with the onChange handler: onChange accepts number | string | boolean |
undefined but meta.events and meta.variables still declare the type as 'string';
update the metadata to reflect the broader union type (or a generic 'any' if
preferred) so emitted event payloads and variable typings match onChange's
signature—locate meta.events and meta.variables in SingleSelectFieldPro.emb.ts
and change their type from 'string' to the union type (number | string | boolean
| undefined) or a compatible type to align with onChange and Value.noFilter()
handling.
🧹 Nitpick comments (1)
src/components/editors/SingleSelectFieldPro/index.tsx (1)
13-21: WidenselectedValuetype to match new supported values.
onChangenow emits boolean/number, butselectedValueis still typed asstring, which prevents passing these values back in and forces unsafe casts.♻️ Suggested type update
type SingleSelectFieldProProps = { dimension: Dimension; optionalSecondDimension?: Dimension; placeholder?: string; results: DataResponse; - selectedValue?: string; + selectedValue?: string | boolean | number; maxOptions?: number; setSearchValue?: (search: string) => void; onChange?: (selectedValue: string | boolean | number | undefined) => void; } & ChartCardHeaderProps;
Comment on lines
+53
to
+64
| const selectedValueIsBoolean = optionalSecondDimension | ||
| ? optionalSecondDimension.nativeType === 'boolean' | ||
| : dimension.nativeType === 'boolean'; | ||
|
|
||
| const safeValue = options.find((option) => { | ||
| let value: string | boolean | undefined = selectedValue; | ||
| if (selectedValueIsBoolean) { | ||
| value = selectedValue === 'true' ? true : selectedValue === 'false' ? false : undefined; | ||
| } | ||
|
|
||
| return option.value === value; | ||
| })?.value; |
There was a problem hiding this comment.
safeValue drops boolean inputs and doesn’t coerce numeric strings.
If selectedValue is already a boolean, the current logic turns it into undefined. Also, numeric selections won’t preselect when selectedValue is a string (common given the selectedValue input). This breaks preselection for boolean and numeric options.
🐛 Suggested fix for boolean + numeric handling
- const selectedValueIsBoolean = optionalSecondDimension
+ const selectedValueIsBoolean = optionalSecondDimension
? optionalSecondDimension.nativeType === 'boolean'
: dimension.nativeType === 'boolean';
+
+ const selectedValueIsNumber = optionalSecondDimension
+ ? optionalSecondDimension.nativeType === 'number'
+ : dimension.nativeType === 'number';
const safeValue = options.find((option) => {
- let value: string | boolean | undefined = selectedValue;
+ let value: string | boolean | number | undefined = selectedValue;
if (selectedValueIsBoolean) {
- value = selectedValue === 'true' ? true : selectedValue === 'false' ? false : undefined;
+ if (selectedValue === true || selectedValue === false) {
+ value = selectedValue;
+ } else if (selectedValue === 'true') {
+ value = true;
+ } else if (selectedValue === 'false') {
+ value = false;
+ } else {
+ value = undefined;
+ }
+ } else if (selectedValueIsNumber && typeof selectedValue === 'string') {
+ const parsed = Number(selectedValue);
+ value = Number.isNaN(parsed) ? undefined : parsed;
}
return option.value === value;
})?.value;🤖 Prompt for AI Agents
In `@src/components/editors/SingleSelectFieldPro/index.tsx` around lines 53 - 64,
The safeValue lookup currently turns an already-boolean selectedValue into
undefined and doesn't handle numeric-string coercion; update the safeValue
computation (the block using selectedValueIsBoolean, optionalSecondDimension,
dimension, selectedValue, and options) to coerce selectedValue into the option
value type instead of forcing undefined: if selectedValueIsBoolean is true,
accept true/false values and also the strings 'true'/'false' (preserve booleans
if already boolean); if the matched option values are numbers, coerce numeric
strings to numbers (Number or parseFloat) before comparison; otherwise compare
using strict equality to find the matching option.value and return that value
for safeValue.
| return option.value === value; | ||
| })?.value; | ||
|
|
||
| console.log('selectedValue', selectedValue, safeValue); |
There was a problem hiding this comment.
Remove the debug console.log.
This will leak user data into logs and adds noise in production.
🤖 Prompt for AI Agents
In `@src/components/editors/SingleSelectFieldPro/index.tsx` at line 66, Remove the
debug console.log that prints selectedValue and safeValue inside the
SingleSelectFieldPro component: locate the stray console.log('selectedValue',
selectedValue, safeValue) and delete it (or replace it with a proper
non-user-data-producing debug log behind an env-level debug flag if you need
runtime diagnostics), ensuring no user data is written to stdout/stderr.
Comment on lines
+110
to
+112
| onChange: (selectedValue: number | string | boolean | undefined) => { | ||
| return { | ||
| value: selectedValue || Value.noFilter(), | ||
| value: selectedValue ?? Value.noFilter(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "SingleSelectFieldPro.emb.ts" | head -5Repository: embeddable-hq/remarkable-pro
Length of output: 145
🏁 Script executed:
cat -n src/components/editors/SingleSelectFieldPro/SingleSelectFieldPro.emb.tsRepository: embeddable-hq/remarkable-pro
Length of output: 4041
🏁 Script executed:
# Search for meta.events, meta.variables, and inputs definitions in the file
rg "meta\.(events|variables|inputs)" src/components/editors/SingleSelectFieldPro/SingleSelectFieldPro.emb.ts -A 3Repository: embeddable-hq/remarkable-pro
Length of output: 54
Update meta.events and meta.variables to match the broader onChange input type.
onChange now accepts number | string | boolean | undefined, but the metadata still declares the event property and variable type as 'string'. While this may work for string values, it'll cause type mismatches if the component receives or emits non-string values. Align the metadata types (lines 57, 65) with the handler's accepted types to avoid downstream confusion.
🤖 Prompt for AI Agents
In `@src/components/editors/SingleSelectFieldPro/SingleSelectFieldPro.emb.ts`
around lines 110 - 112, The meta declarations for this component are out of sync
with the onChange handler: onChange accepts number | string | boolean |
undefined but meta.events and meta.variables still declare the type as 'string';
update the metadata to reflect the broader union type (or a generic 'any' if
preferred) so emitted event payloads and variable typings match onChange's
signature—locate meta.events and meta.variables in SingleSelectFieldPro.emb.ts
and change their type from 'string' to the union type (number | string | boolean
| undefined) or a compatible type to align with onChange and Value.noFilter()
handling.
mad-raccoon
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this pull-request needed?
This PR allows the Single and Multi Selectfield to work with strings, numbers and booleans.
This is an hotfix - we will need to go back on Remarkable ui and improve the types later.
If merged, please make sure it is properly tested.
After merged, a Version PR will be created on the Pull Requests tab. After that is approved and merged, a new version will be created, and we can tell the client this was sorted.
Test evidence
Screen.Recording.2026-01-21.at.16.11.27.mov
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.