-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Playwright stage 1 - add data-testid attribute #7894
Changes from 5 commits
5cfc719
a85326e
663e6a7
cf91740
ced0a97
bdde13c
b82fecb
6e77074
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,9 +17,11 @@ export const ActivityRow = ({ | |
children, | ||
onClick, | ||
disabled, | ||
testId, | ||
}: React.PropsWithChildren<{ | ||
onClick?: (event: React.MouseEvent<HTMLDivElement>) => void; | ||
disabled?: boolean; | ||
testId?: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Add JSDoc comment to explain the purpose of the testId prop |
||
}>) => { | ||
const handleClick = (event: React.MouseEvent<HTMLDivElement>) => { | ||
if (disabled !== true) { | ||
|
@@ -28,7 +30,11 @@ export const ActivityRow = ({ | |
}; | ||
|
||
return ( | ||
<StyledRowContent onClick={handleClick} clickable={disabled !== true}> | ||
<StyledRowContent | ||
onClick={handleClick} | ||
clickable={disabled !== true} | ||
data-testid={testId} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Ensure consistent naming convention for test IDs across the application |
||
> | ||
{children} | ||
</StyledRowContent> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,7 +125,7 @@ export const AttachmentRow = ({ attachment }: { attachment: Attachment }) => { | |
|
||
return ( | ||
<FieldContext.Provider value={fieldContext as GenericFieldContextType}> | ||
<ActivityRow disabled> | ||
<ActivityRow disabled testId="attachment-row"> | ||
<StyledLeftContent> | ||
<AttachmentIcon attachmentType={attachment.type} /> | ||
{isEditing ? ( | ||
|
@@ -135,6 +135,7 @@ export const AttachmentRow = ({ attachment }: { attachment: Attachment }) => { | |
onBlur={handleOnBlur} | ||
autoFocus | ||
onKeyDown={handleOnKeyDown} | ||
dataTestId="file-name-input" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using consistent naming: 'testId' vs 'dataTestId' |
||
/> | ||
) : ( | ||
<StyledLinkContainer> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,6 +96,7 @@ export const Attachments = ({ | |
title="Add file" | ||
variant="secondary" | ||
onClick={handleUploadFileClick} | ||
dataTestId="add-file-button" | ||
/> | ||
</AnimatedPlaceholderEmptyContainer> | ||
)} | ||
|
@@ -121,6 +122,7 @@ export const Attachments = ({ | |
variant="secondary" | ||
title="Add file" | ||
onClick={handleUploadFileClick} | ||
dataTestId="add-file-button" | ||
></Button> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. syntax: Remove empty closing tag |
||
} | ||
/> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ export const AddTaskButton = ({ | |
targetableObjects: activityTargetableObjects, | ||
}) | ||
} | ||
data-testid="add-task-button" | ||
></Button> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: consider adding a child element or text to the Button component |
||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,6 +93,7 @@ export const TaskRow = ({ task }: { task: Task }) => { | |
onClick={() => { | ||
openActivityRightDrawer(task.id); | ||
}} | ||
testId="task-row" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using a more specific testId, e.g., |
||
> | ||
<StyledLeftSideContainer> | ||
<StyledCheckboxContainer | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -165,6 +165,10 @@ export const RecordDetailRelationSection = ({ | |
return ( | ||
<RecordDetailSection> | ||
<RecordDetailSectionHeader | ||
testId={fieldDefinition.label | ||
.toLowerCase() | ||
.replace(' ', '-') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Replace ' ' with a regex to handle multiple spaces |
||
.concat('-header')} | ||
title={fieldDefinition.label} | ||
link={ | ||
isToManyObjects | ||
|
@@ -191,6 +195,10 @@ export const RecordDetailRelationSection = ({ | |
className="displayOnHover" | ||
Icon={isToOneObject ? IconPencil : IconPlus} | ||
accent="tertiary" | ||
testId={fieldDefinition.label | ||
.toLowerCase() | ||
.replace(' ', '-') | ||
.concat('-dropdown')} | ||
/> | ||
} | ||
dropdownComponents={ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,7 +99,11 @@ export const SettingsAccountsBlocklistInput = ({ | |
)} | ||
/> | ||
</StyledLinkContainer> | ||
<Button title="Add to blocklist" type="submit" /> | ||
<Button | ||
title="Add to blocklist" | ||
type="submit" | ||
dataTestId="add-blocklist-button" | ||
/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using camelCase for dataTestId to maintain consistency with React naming conventions |
||
</StyledContainer> | ||
</form> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,7 @@ export const SettingsDataModelFieldIconLabelForm = ({ | |
disabled={disabled} | ||
maxLength={maxLength} | ||
fullWidth | ||
dataTestId="field-name-input" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using a consistent naming convention for data-testid attributes across the project |
||
/> | ||
)} | ||
/> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,7 @@ export const SettingsDataModelFieldToggle = ({ | |
value={value} | ||
onChange={onChange} | ||
toggleSize="small" | ||
testId="relative-datetime-toggle" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using a more specific testId. 'relative-datetime-toggle' might not accurately describe this toggle's purpose in all contexts. |
||
/> | ||
</StyledGroup> | ||
</StyledContainer> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -146,19 +146,22 @@ export const SettingsDataModelFieldNumberDecimalsInput = ({ | |
onClick={handleDecrementCounter} | ||
Icon={IconMinus} | ||
disabled={disabled} | ||
dataTestId="decrement-decimals-button" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using a consistent naming convention for data-testid attributes across the codebase |
||
/> | ||
<StyledTextInput | ||
name="decimals" | ||
fullWidth | ||
value={value.toString()} | ||
onChange={(value) => handleTextInputChange(value)} | ||
disabled={disabled} | ||
dataTestId="decimals-number-input" | ||
/> | ||
<StyledControlButton | ||
variant="secondary" | ||
onClick={handleIncrementCounter} | ||
Icon={IconPlus} | ||
disabled={disabled} | ||
dataTestId="increment-decimals-button" | ||
/> | ||
</StyledCounterControlsIcons> | ||
</StyledCounterInnerContainer> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for example why do we need this one? can't we select based on entry.label directly (this will select the label but I guess it's still fine to trigger DOM events on parents)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one isn't necessary and can be located using text, I wanted to make life a bit easier but I'll refactor it, re-check my changes and delete attributes from places which can be located using text