Skip to content

Commit 1b75a10

Browse files
fix: show whitespace warnings for IN operator comma-separated values (#6203)
1 parent f95a4f6 commit 1b75a10

File tree

7 files changed

+208
-105
lines changed

7 files changed

+208
-105
lines changed

frontend/web/components/modals/CreateSegment.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ const CreateSegment: FC<CreateSegmentType> = ({
498498
</Row>
499499
}
500500
>
501-
<div className='my-4 col-lg-8'>
501+
<div className='my-4 col-lg-8 mx-auto'>
502502
<CreateSegmentRulesTabForm
503503
is4Eyes={is4Eyes}
504504
onCreateChangeRequest={onCreateChangeRequest}
@@ -526,7 +526,7 @@ const CreateSegment: FC<CreateSegmentType> = ({
526526
</div>
527527
</TabItem>
528528
<TabItem tabLabel='Features'>
529-
<div className='my-4 col-lg-8'>
529+
<div className='my-4 col-lg-8 mx-auto'>
530530
<AssociatedSegmentOverrides
531531
onUnsavedChange={() => {
532532
setValueChanged(true)
@@ -539,7 +539,7 @@ const CreateSegment: FC<CreateSegmentType> = ({
539539
</div>
540540
</TabItem>
541541
<TabItem tabLabel='Users'>
542-
<div className='my-4 col-lg-8'>
542+
<div className='my-4 col-lg-8 mx-auto'>
543543
<CreateSegmentUsersTabContent
544544
projectId={projectId}
545545
environmentId={environmentId}
@@ -566,7 +566,7 @@ const CreateSegment: FC<CreateSegmentType> = ({
566566
</Row>
567567
}
568568
>
569-
<div className='my-4 col-lg-8'>{MetadataTab}</div>
569+
<div className='my-4 col-lg-8 mx-auto'>{MetadataTab}</div>
570570
</TabItem>
571571
)}
572572
</Tabs>

frontend/web/components/segments/Rule/components/RuleConditionRow.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ const RuleConditionRow: React.FC<RuleConditionRowProps> = ({
149149
style={{ width: '135px' }}
150150
projectId={projectId}
151151
showEnvironmentDropdown={showEnvironmentDropdown}
152+
operator={operator}
152153
onChange={(value: string) => {
153154
setRuleProperty(ruleIndex, 'value', {
154155
value:

frontend/web/components/segments/Rule/components/RuleConditionValueInput.tsx

Lines changed: 16 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
import React from 'react'
22
import Input from 'components/base/forms/Input'
33
import Icon from 'components/Icon'
4-
import InputGroup from 'components/base/forms/InputGroup'
5-
import Button from 'components/base/forms/Button'
64
import Utils from 'common/utils/utils'
75

86
import { getDarkMode } from 'project/darkMode'
9-
import ModalHR from 'components/modals/ModalHR'
107
import EnvironmentSelectDropdown from './EnvironmentSelectDropdown'
8+
import TextAreaModal from './TextAreaModal'
9+
import { checkWhitespaceIssues } from '../utils'
1110

1211
type RuleConditionValueInputProps = {
1312
'data-test'?: string
@@ -20,108 +19,25 @@ type RuleConditionValueInputProps = {
2019
isValid?: boolean
2120
projectId?: number
2221
showEnvironmentDropdown?: boolean
23-
}
24-
25-
const TextAreaModal = ({
26-
disabled,
27-
isValid,
28-
onChange,
29-
placeholder,
30-
readOnly,
31-
style,
32-
value,
33-
}: RuleConditionValueInputProps) => {
34-
const [textAreaValue, setTextAreaValue] = React.useState(value)
35-
36-
return (
37-
<div>
38-
<div className='modal-body'>
39-
<InputGroup
40-
id='rule-value-textarea'
41-
data-test='rule-value-textarea'
42-
value={textAreaValue}
43-
inputProps={{
44-
style: style,
45-
}}
46-
isValid={isValid}
47-
onChange={(e: InputEvent) => {
48-
const value = Utils.safeParseEventValue(e)
49-
setTextAreaValue(value.replace(/\n/g, ''))
50-
}}
51-
type='text'
52-
className='w-100'
53-
readOnly={readOnly}
54-
placeholder={placeholder}
55-
disabled={disabled}
56-
textarea
57-
/>
58-
</div>
59-
<ModalHR />
60-
<div className='modal-footer'>
61-
<Button
62-
className='mr-2'
63-
theme='secondary'
64-
id='rule-value-textarea-cancel'
65-
data-tests='rule-value-textarea-cancel'
66-
onClick={closeModal2}
67-
>
68-
Cancel
69-
</Button>
70-
<Button
71-
type='button'
72-
id='rule-value-textarea-save'
73-
data-tests='rule-value-textarea-save'
74-
onClick={() => {
75-
const event = new InputEvent('input', { bubbles: true })
76-
Object.defineProperty(event, 'target', {
77-
value: { value: textAreaValue },
78-
writable: false,
79-
})
80-
const value = Utils.getTypedValue(Utils.safeParseEventValue(event))
81-
onChange?.(value)
82-
closeModal2()
83-
}}
84-
>
85-
Apply
86-
</Button>
87-
</div>
88-
</div>
89-
)
22+
operator?: string
9023
}
9124

9225
const RuleConditionValueInput: React.FC<RuleConditionValueInputProps> = ({
9326
isValid,
9427
onChange,
28+
operator,
9529
projectId,
9630
showEnvironmentDropdown,
9731
value,
9832
...props
9933
}) => {
100-
const hasLeadingWhitespace = typeof value === 'string' && /^\s/.test(value)
101-
const hasTrailingWhitespace = typeof value === 'string' && /\s$/.test(value)
102-
const isOnlyWhitespace =
103-
typeof value === 'string' && value.length >= 1 && value.trim() === ''
104-
const hasBothLeadingAndTrailingWhitespace =
105-
hasLeadingWhitespace && hasTrailingWhitespace
106-
const hasWarning =
107-
hasLeadingWhitespace ||
108-
hasTrailingWhitespace ||
109-
hasBothLeadingAndTrailingWhitespace ||
110-
isOnlyWhitespace
34+
const whitespaceCheck = checkWhitespaceIssues(value, operator)
35+
const hasWarning = !!whitespaceCheck
11136
const isLongText = String(value).length >= 10
11237

11338
const validate = () => {
114-
if (isOnlyWhitespace) {
115-
return 'This value is only whitespaces'
116-
}
117-
if (hasBothLeadingAndTrailingWhitespace) {
118-
return 'This value starts and ends with whitespaces'
119-
}
120-
if (hasLeadingWhitespace) {
121-
return 'This value starts with whitespaces'
122-
}
123-
if (hasTrailingWhitespace) {
124-
return 'This value ends with whitespaces'
39+
if (whitespaceCheck?.message) {
40+
return whitespaceCheck.message
12541
}
12642
if (isLongText) {
12743
return 'Click to edit text in a larger area'
@@ -146,6 +62,8 @@ const RuleConditionValueInput: React.FC<RuleConditionValueInputProps> = ({
14662
<Input
14763
type='text'
14864
data-test={props['data-test']}
65+
name='rule-condition-value-input'
66+
aria-label='Rule condition value input'
14967
value={value}
15068
inputClassName={
15169
showIcon ? `pr-5 ${hasWarning ? 'border-warning' : ''}` : ''
@@ -161,19 +79,16 @@ const RuleConditionValueInput: React.FC<RuleConditionValueInputProps> = ({
16179
<Tooltip
16280
title={
16381
<div
164-
className={`flex ${
82+
className={`rule-value-icon flex ${
16583
isDarkMode ? 'bg-white' : 'bg-black'
166-
} bg-opacity-10 rounded-2 p-1 ${
167-
hasWarning ? '' : 'cursor-pointer'
168-
}`}
84+
} bg-opacity-10 rounded-2 p-1 cursor-pointer`}
16985
onClick={() => {
170-
if (hasWarning) return
17186
openModal2(
17287
'Edit Value',
173-
<TextAreaModal
174-
value={value}
175-
onChange={onChange}
176-
isValid={isValid}
88+
<TextAreaModal
89+
value={value}
90+
onChange={onChange}
91+
operator={operator}
17792
/>,
17893
)
17994
}}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import React, { useState, FC } from 'react'
2+
import InputGroup from 'components/base/forms/InputGroup'
3+
import Button from 'components/base/forms/Button'
4+
import Icon from 'components/Icon'
5+
import Utils from 'common/utils/utils'
6+
import ModalHR from 'components/modals/ModalHR'
7+
import { checkWhitespaceIssues } from '../utils'
8+
9+
type TextAreaModalProps = {
10+
value: string | number | boolean
11+
onChange?: (value: string) => void
12+
operator?: string
13+
}
14+
15+
const TextAreaModal: FC<TextAreaModalProps> = ({ onChange, value, operator }) => {
16+
const [textAreaValue, setTextAreaValue] = useState(value)
17+
const whitespaceCheck = checkWhitespaceIssues(textAreaValue, operator)
18+
const hasWarning = !!whitespaceCheck
19+
20+
return (
21+
<div>
22+
<div className='modal-body d-flex flex-column gap-1'>
23+
<InputGroup
24+
id='rule-value-textarea'
25+
data-test='rule-value-textarea'
26+
value={textAreaValue}
27+
onChange={(e: InputEvent) => {
28+
const value = Utils.safeParseEventValue(e)
29+
setTextAreaValue(value.replace(/\n/g, ''))
30+
}}
31+
noMargin
32+
inputProps={{
33+
className: hasWarning ? 'border-warning' : '',
34+
}}
35+
type='text'
36+
className='w-100'
37+
textarea
38+
/>
39+
<div
40+
style={{
41+
minHeight: '20px',
42+
transition: 'opacity 0.25s ease-in-out',
43+
opacity: hasWarning ? 1 : 0,
44+
}}
45+
>
46+
{hasWarning && (
47+
<div className='text-warning d-flex align-items-start gap-2'>
48+
<span className='d-flex' style={{ alignSelf: 'center' }}>
49+
<Icon name='warning' width={16} height={16} />
50+
</span>
51+
<span>{whitespaceCheck?.message}</span>
52+
</div>
53+
)}
54+
</div>
55+
</div>
56+
<ModalHR />
57+
<div className='modal-footer'>
58+
<Button
59+
className='mr-2'
60+
theme='secondary'
61+
id='rule-value-textarea-cancel'
62+
data-tests='rule-value-textarea-cancel'
63+
onClick={closeModal2}
64+
>
65+
Cancel
66+
</Button>
67+
<Button
68+
type='button'
69+
id='rule-value-textarea-save'
70+
data-tests='rule-value-textarea-save'
71+
onClick={() => {
72+
const event = new InputEvent('input', { bubbles: true })
73+
Object.defineProperty(event, 'target', {
74+
value: { value: textAreaValue },
75+
writable: false,
76+
})
77+
const value = Utils.getTypedValue(Utils.safeParseEventValue(event))
78+
onChange?.(value)
79+
closeModal2()
80+
}}
81+
>
82+
Apply
83+
</Button>
84+
</div>
85+
</div>
86+
)
87+
}
88+
89+
export default TextAreaModal
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { checkWhitespaceIssues } from './whitespaceValidation'
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/**
2+
* Checks for whitespace issues in rule condition values, particularly for IN operator
3+
* @param value - The value to check for whitespace issues
4+
* @param operator - The operator being used (e.g., 'IN')
5+
* @returns Object with warning message if issues found, null otherwise
6+
*/
7+
export const checkWhitespaceIssues = (
8+
value: string | number | boolean,
9+
operator?: string,
10+
): { message: string } | null => {
11+
if (operator !== 'IN') return null
12+
if (typeof value !== 'string') return null
13+
14+
const LEADING_WHITESPACE = /^\s/
15+
const TRAILING_WHITESPACE = /\s$/
16+
17+
if (value.length >= 1 && value.trim() === '') {
18+
return { message: 'This value is only whitespaces' }
19+
}
20+
21+
const items = value.split(',')
22+
23+
if (items.length > 1) {
24+
const counts = items.reduce(
25+
(acc, item) => {
26+
const hasLeading = LEADING_WHITESPACE.test(item)
27+
const hasTrailing = TRAILING_WHITESPACE.test(item)
28+
29+
if (hasLeading && hasTrailing) acc.both++
30+
else if (hasLeading) acc.leading++
31+
else if (hasTrailing) acc.trailing++
32+
33+
return acc
34+
},
35+
{ both: 0, leading: 0, trailing: 0 },
36+
)
37+
38+
const totalIssues = counts.both + counts.leading + counts.trailing
39+
const hasMultipleIssues =
40+
[counts.both > 0, counts.leading > 0, counts.trailing > 0].filter(Boolean)
41+
.length > 1
42+
43+
if (totalIssues > 0) {
44+
if (hasMultipleIssues) {
45+
return {
46+
message: `${totalIssues} item(s) have whitespace issues`,
47+
}
48+
}
49+
50+
if (counts.both > 0) {
51+
return {
52+
message: `${counts.both} item(s) have leading and trailing whitespaces`,
53+
}
54+
}
55+
if (counts.leading > 0) {
56+
return {
57+
message: `${counts.leading} item(s) have leading whitespaces`,
58+
}
59+
}
60+
if (counts.trailing > 0) {
61+
return {
62+
message: `${counts.trailing} item(s) have trailing whitespaces`,
63+
}
64+
}
65+
}
66+
}
67+
68+
const hasLeading = LEADING_WHITESPACE.test(value)
69+
const hasTrailing = TRAILING_WHITESPACE.test(value)
70+
71+
if (hasLeading && hasTrailing) {
72+
return {
73+
message: 'This value starts and ends with whitespaces',
74+
}
75+
}
76+
if (hasLeading) {
77+
return { message: 'This value starts with whitespaces' }
78+
}
79+
if (hasTrailing) {
80+
return { message: 'This value ends with whitespaces' }
81+
}
82+
83+
return null
84+
}

0 commit comments

Comments
 (0)