-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: updated the design for Messaging Queue - summary section #6319
Conversation
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Reviewed everything up to 8d98f1c in 35 seconds
More details
- Looked at
367
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
7
drafted comments based on config settings.
1. frontend/src/constants/query.ts:40
- Draft comment:
The addition ofmqServiceView
toQueryParams
is consistent with its usage in the code for managing URL query parameters related to messaging queue views. - Reason this comment was not posted:
Confidence changes required:0%
ThemqServiceView
query parameter is added to theQueryParams
enum, which is used in the code to manage URL query parameters. This addition seems consistent with the changes in the code wheremqServiceView
is used to determine the view type in the messaging queue details page.
2. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:44
- Draft comment:
Ensure that thedefaultValue
does not override thevalue
prop set bymqServiceView
from the URL query. Consider removingdefaultValue
ifmqServiceView
is always expected to be present. - Reason this comment was not posted:
Confidence changes required:50%
ThemqServiceView
query parameter is used to set the value of the Select component, which is a good practice for maintaining state consistency with the URL. However, the default value is also set, which might override the URL parameter if not handled properly.
3. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:25
- Draft comment:
TheredirectToDetailsPage
function is well-implemented to handle navigation with the correctmqServiceView
parameter. Ensure that all possiblecallerView
values are valid and handled appropriately in the details page. - Reason this comment was not posted:
Confidence changes required:0%
TheredirectToDetailsPage
function is used to navigate to the details page with a specific view type. This function is used consistently across different buttons to ensure the correct view is displayed based on user interaction.
4. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:139
- Draft comment:
Avoid using inline styles for the Button component. Consider using CSS classes or styled components instead. This applies to other Button components in this file as well. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment seems to be incorrect because there are no inline styles applied to the Button components in the provided code. The Button components are using a 'type' prop, which is not an inline style. Therefore, the comment does not seem to be applicable to the changes made in this diff.
I might be missing some context about how the Button component is styled elsewhere, but based on the provided code, there are no inline styles present. It's possible that the comment is a generic suggestion not specific to this diff.
Given the code provided, there is no evidence of inline styles being used for the Button component. The comment does not seem relevant to the changes made in this diff.
The comment should be deleted as it does not apply to the changes made in this diff. There are no inline styles used for the Button component in the provided code.
5. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:157
- Draft comment:
Avoid using inline styles for the Button component. Consider using CSS classes or styled components instead. This applies to other Button components in this file as well. - Reason this comment was not posted:
Marked as duplicate.
6. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:175
- Draft comment:
Avoid using inline styles for the Button component. Consider using CSS classes or styled components instead. This applies to other Button components in this file as well. - Reason this comment was not posted:
Marked as duplicate.
7. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:193
- Draft comment:
Avoid using inline styles for the Button component. Consider using CSS classes or styled components instead. This applies to other Button components in this file as well. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_0W4Xhs95fjuz5zcX
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
2 similar comments
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Incremental review on c1592c3 in 27 seconds
More details
- Looked at
28
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:158
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This applies to thedisabled
attribute in the Button components. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_Mh450aPeMlGuc6qj
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
7a8e7a5
to
352b4b0
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Incremental review on 352b4b0 in 28 seconds
More details
- Looked at
28
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:158
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This is also applicable at lines 177 and 196. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_NeISPKIuTlmjft4E
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
352b4b0
to
41d4cad
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Incremental review on 41d4cad in 41 seconds
More details
- Looked at
28
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:158
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This applies to thedisabled
attribute in the Button components here and elsewhere in this file. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_stbGsX2e3R0LPREX
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
41d4cad
to
7f5a78a
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Incremental review on 7f5a78a in 37 seconds
More details
- Looked at
28
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:158
- Draft comment:
Avoid using inline styles for theButton
component. Use external stylesheets, CSS classes, or styled components instead. This applies to other instances of inline styles in this file as well. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_JkwmeP6fpw99URDZ
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
7f5a78a
to
6186625
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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 48b49e1 in 21 seconds
More details
- Looked at
16
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:65
- Draft comment:
Consider providing a fallback value formqServiceView
in case it isnull
or undefined to prevent potential errors.
value={mqServiceView ? mqServiceView : MessagingQueuesViewType.consumerLag.value}
- Reason this comment was not posted:
Comment was on unchanged code.
2. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:59
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_Cb2GVGrZ7gdLucP5
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
@SagarRajput-7, can you check this? there seems to be some issue while switching between views. even if i select partition latency view, it keeps consumer lag view as active consumer.lag.view.-.switching.views.issue.mov |
Fixed - Screen.Recording.2024-11-06.at.11.48.30.AM.mov |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Incremental review on 1463c56 in 49 seconds
More details
- Looked at
52
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:52
- Draft comment:
Ensurehistory.location.search
is defined before using it innew URLSearchParams
. This prevents potential errors ifsearch
is undefined. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_qsW65OiB3etTbDNM
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
6a8c8ae
1463c56
to
6a8c8ae
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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 6a8c8ae in 45 seconds
More details
- Looked at
52
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:51
- Draft comment:
Ensurehistory.location.search
is defined before using it innew URLSearchParams
. This prevents potential errors ifhistory.location.search
is undefined. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:34
- Draft comment:
RenamesetproducerLatencyOption
tosetProducerLatencyOption
to follow camelCase naming convention. This applies to all instances of this function. - Reason this comment was not posted:
Confidence changes required:50%
ThesetproducerLatencyOption
function is not following camelCase convention. It should besetProducerLatencyOption
to maintain consistency and readability.
3. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:83
- Draft comment:
EnsuremqServiceView
has a fallback value to prevent potential issues if it'sundefined
. Consider usingmqServiceView || MessagingQueuesViewType.consumerLag.value
. - Reason this comment was not posted:
Comment did not seem useful.
4. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:34
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_CfnHvrCDlfLGOR77
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary
Related Issues / PR's
Screenshots
Before -
Updated design -
Interaction and redirection -
Screen.Recording.2024-10-30.at.2.23.38.PM.mov
Affected Areas and Manually Tested Areas
Important
Updated Messaging Queue summary section with new Kafka metric views and enhanced UI components.
messagingQueuesKafkaOverview.json
with new titles and descriptions for consumer lag, producer latency, partition latency, and drop rate views.mqServiceView
toQueryParams
inquery.ts
for handling view-specific queries.MQDetailPage.tsx
to usemqServiceView
for view selection and added drop rate options.MessagingQueues.tsx
to include new overview cards for consumer, producer, partition, and drop rate views with redirection.MessagingQueues.styles.scss
for new layout and card designs.MessagingQueuesUtils.ts
to include new view types inMessagingQueuesViewType
.This description was created by for 6a8c8ae. It will automatically update as commits are pushed.