Skip to content
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

fix: kafka - misc fix and features #6379

Merged
merged 6 commits into from
Nov 7, 2024
Merged

fix: kafka - misc fix and features #6379

merged 6 commits into from
Nov 7, 2024

Conversation

SagarRajput-7
Copy link
Contributor

@SagarRajput-7 SagarRajput-7 commented Nov 6, 2024

Summary

Related Issues / PR's

https://github.com/SigNoz/engineering-pod/issues/1914

Screenshots

NA

Affected Areas and Manually Tested Areas


Important

Enhance Kafka messaging queue UI and functionality with improved component structure, user interaction handling, and styling adjustments.

  • UI Restructuring:
    • Refactor MQDetailPage.tsx for improved component structure and conditional rendering.
    • Simplify checkValidityOfDetailConfigs() logic in MQDetails.tsx.
  • Functionality Enhancements:
    • Add redirection logic in AttributeCheckList.tsx for cloud users based on parentTitle.
    • Adjust pagination in MQTables.tsx to show 10 items per page instead of 20.
  • Styling and Miscellaneous:
    • Add cursor styles in MessagingQueueHealthCheck.styles.scss for better UX.
    • Minor CSS adjustments in MessagingQueues.styles.scss for consistent styling.

This description was created by Ellipsis for 723b211. It will automatically update as commits are pushed.

Copy link

github-actions bot commented Nov 6, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added bug Something isn't working enhancement New feature or request docs required labels Nov 6, 2024
Copy link

github-actions bot commented Nov 6, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 27e62b0 in 1 minute and 1 seconds

More details
  • Looked at 314 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:103
  • Draft comment:
    Refactored conditional rendering logic improves readability by reducing nested elements. Good change!
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The refactoring of the conditional rendering logic in MQDetailPage.tsx improves readability by reducing nested elements. The logic remains the same, so this change is beneficial.
2. frontend/src/pages/MessagingQueues/MQDetails/MQTables/MQTables.tsx:158
  • Draft comment:
    Changed pagination from 20 to 10 items per page. This could be intended to improve performance or readability.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The change in pagination configuration from 20 to 10 items per page in MQTables.tsx is a design decision that could affect user experience. It might be intended to improve performance or readability.
3. frontend/src/pages/MessagingQueues/MessagingQueueHealthCheck/AttributeCheckList.tsx:62
  • Draft comment:
    Encapsulation of redirection logic in handleRedirection is a good practice. It handles both cloud and non-cloud users appropriately.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The addition of handleRedirection function in AttributeCheckList.tsx is a good encapsulation of the redirection logic. It handles both cloud and non-cloud users appropriately.
4. frontend/src/pages/MessagingQueues/MessagingQueueHealthCheck/AttributeCheckList.tsx:223
  • Draft comment:
    The use of useEffect with filter and onboardingStatusResponses as dependencies is correct. It ensures that the tree data is updated whenever these values change.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The use of useEffect with filter and onboardingStatusResponses as dependencies is correct. It ensures that the tree data is updated whenever these values change.
5. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:106
  • Draft comment:
    Avoid using inline styles for the Typography.Text component. Use external stylesheets or styled components instead. This applies to other instances in this file as well.
  • Reason this comment was not posted:
    Comment was on unchanged code.
6. frontend/src/pages/MessagingQueues/MQDetails/MQDetails.tsx:116
  • Draft comment:
    Avoid using inline styles for the Radio.Button component. Use external stylesheets or styled components instead. This applies to other instances in this file as well.
  • Reason this comment was not posted:
    Comment was on unchanged code.
7. frontend/src/pages/MessagingQueues/MQDetails/MQTables/MQTables.tsx:155
  • Draft comment:
    Avoid using inline styles for the Typography.Link component. Use external stylesheets or styled components instead. This applies to other instances in this file as well.
  • Reason this comment was not posted:
    Comment was on unchanged code.
8. frontend/src/pages/MessagingQueues/MessagingQueueHealthCheck/AttributeCheckList.tsx:95
  • Draft comment:
    Avoid using inline styles for the div element. Use external stylesheets or styled components instead. This applies to other instances in this file as well.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_ajPxFWtQH6cCcLft


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 723b211 in 17 seconds

More details
  • Looked at 38 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MessagingQueueHealthCheck/AttributeCheckList.tsx:62
  • Draft comment:
    Removing the console.log statement is a good practice for production code to avoid unnecessary logging.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The removal of the console.log statement is appropriate for production code, as it avoids unnecessary logging.

Workflow ID: wflow_EduVV5hbjkTmIX2l


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

github-actions bot commented Nov 7, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

ahmadshaheer
ahmadshaheer previously approved these changes Nov 7, 2024
YounixM
YounixM previously approved these changes Nov 7, 2024
@SagarRajput-7 SagarRajput-7 removed enhancement New feature or request docs required labels Nov 7, 2024
@SagarRajput-7 SagarRajput-7 changed the title feat: kafka - misc fix and features fix: kafka - misc fix and features Nov 7, 2024
Copy link

github-actions bot commented Nov 7, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link

github-actions bot commented Nov 7, 2024

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the enhancement New feature or request label Nov 7, 2024
@SagarRajput-7 SagarRajput-7 merged commit fdc54a6 into develop Nov 7, 2024
16 of 17 checks passed
@SagarRajput-7 SagarRajput-7 deleted the kafka-misc-fixes branch November 7, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs not required enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants