-
Notifications
You must be signed in to change notification settings - Fork 1.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
chore: fix filter editing issue in infra monitoring #6961
Conversation
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.
👍 Looks good to me! Reviewed everything up to 4980b5a in 20 seconds
More details
- Looked at
88
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/index.tsx:294
- Draft comment:
Typo in comment: 'monitoiring' should be 'monitoring'. - Reason this comment was not posted:
Confidence changes required:10%
The typo in the comment could lead to confusion.
2. frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/index.tsx:395
- Draft comment:
Typo in variable name: 'dynamicPlacholder' should be 'dynamicPlaceholder'. - Reason this comment was not posted:
Confidence changes required:20%
The typo in the comment could lead to confusion.
3. frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/index.tsx:395
- Draft comment:
Avoid using inline styles in React components. Consider using external stylesheets, CSS classes, or styled components instead. This is also applicable to other inline styles in this file. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_D5dSXNxQJE6BsBE8
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
4980b5a
to
e20652a
Compare
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.
👍 Looks good to me! Incremental review on e20652a in 20 seconds
More details
- Looked at
88
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/index.tsx:209
- Draft comment:
Consider usingtoggleEditMode(false)
consistently to ensure editing mode is properly toggled off. This is already done in some places but should be consistent across all relevant handlers. - Reason this comment was not posted:
Confidence changes required:50%
The function toggleEditMode is defined but not used consistently. It should be used in all places where editing mode is toggled.
2. frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/index.tsx:401
- Draft comment:
Consider usingtoggleEditMode(false)
consistently to ensure editing mode is properly toggled off. This is already done in some places but should be consistent across all relevant handlers. - Reason this comment was not posted:
Confidence changes required:50%
The function toggleEditMode is defined but not used consistently. It should be used in all places where editing mode is toggled.
3. frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/index.tsx:395
- Draft comment:
Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. This is also applicable in other parts of the code where inline styles are used. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_C8Is6fPqc1X6h25C
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
e20652a
to
24db346
Compare
frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/index.tsx
Show resolved
Hide resolved
24db346
to
354d82f
Compare
Summary
If a quick filters was already applied and we'd try to edit it later, the filter would get temporarily removed from the query and the list would be unfiltered. Fixed the issue here.
Related Issues / PR's
Fixes #6884
Screenshots
NA
Affected Areas and Manually Tested Areas
Infra Monitoring
Important
Fixes filter editing issue in
QueryBuilderSearch
for infra monitoring by managing editing state and conditionally triggeringonChange
.isEditingTag
state inQueryBuilderSearch
to track editing status.onChange
logic to only trigger when editing is complete in infra monitoring mode.toggleEditMode
function to manage editing state transitions.onTagRender
,onInputKeyDownHandler
, andonBlur
to usetoggleEditMode
for managing edit state.onChangeHandler
anduseEffect
for handling tag filters.This description was created by
for e20652a. It will automatically update as commits are pushed.