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

feat: updated the settings page by adding menu list instead of tabs #6116

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

rahulkeswani101
Copy link
Contributor

@rahulkeswani101 rahulkeswani101 commented Oct 3, 2024

Summary

Related Issues / PR's

Screenshots

NA

Affected Areas and Manually Tested Areas


Important

Replaces settings page tabs with a menu list, updates Docker setup, enhances alerting rules, and improves template processing.

  • Settings Page:
    • Replaces tabs with a menu list in SettingsPage.
    • Updates routing for USER_SETTINGS in routes.ts and constants/routes.ts.
    • Adds SettingsNavItems component for menu navigation.
    • Updates GeneralSettings to use new menu layout.
    • Adds styles for menu layout in Settings.styles.scss.
      This description was created by Ellipsis for 391a687. It will automatically update as commits are pushed.

Copy link

github-actions bot commented Oct 3, 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 Oct 3, 2024
@rahulkeswani101 rahulkeswani101 marked this pull request as draft October 3, 2024 15:12
@rahulkeswani101 rahulkeswani101 marked this pull request as ready for review October 8, 2024 05:17
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.

❌ Changes requested. Reviewed everything up to b5131e2 in 54 seconds

More details
  • Looked at 673 lines of code in 18 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. frontend/src/AppRoutes/pageComponents.ts:140
  • Draft comment:
    The webpackChunkName comment should be updated to reflect the new import path.
() => import(/* webpackChunkName: "Settings" */ 'pages/Settings'),
  • Reason this comment was not posted:
    Marked as duplicate.
2. frontend/src/container/GeneralSettings/GeneralSettings.tsx:602
  • Draft comment:
    Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. This applies to other instances in this file as well.
  • Reason this comment was not posted:
    Marked as duplicate.
3. frontend/src/pages/Settings/index.tsx:40
  • Draft comment:
    Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. This applies to other instances in this file as well.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_CFBPluCmW81Sg3R0


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.

@@ -138,7 +138,7 @@ export const APIKeys = Loadable(
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The webpackChunkName comment should be updated to reflect the new import path.

Suggested change
);
() => import(/* webpackChunkName: "Settings" */ 'pages/Settings'),

@@ -524,7 +526,7 @@ function GeneralSettings({
) {
return (
<Fragment key={category.name}>
<Col xs={22} xl={11} key={category.name} style={{ margin: '0.5rem' }}>
<Col xs={22} key={category.name} style={{ margin: '0.5rem' }}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. This applies to other instances in this file as well.

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 391a687 in 2 minutes and 15 seconds

More details
  • Looked at 5803 lines of code in 98 files
  • Skipped 2 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. pkg/query-service/app/http_handler.go:2486
  • Draft comment:
    The onboardProducers, onboardConsumers, and onboardKafka functions have repetitive code for handling attributes and messages. Consider refactoring to reduce redundancy.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The onboardProducers, onboardConsumers, and onboardKafka functions in pkg/query-service/app/http_handler.go have repetitive code for handling attributes and messages. This can be refactored to reduce redundancy and improve maintainability.
2. pkg/query-service/tests/integration/filter_suggestions_test.go:141
  • Draft comment:
    Consider adding comments to explain the purpose of each section in the TestResourceAttribsRankedHigherInLogsFilterSuggestions function for better maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The TestResourceAttribsRankedHigherInLogsFilterSuggestions function in pkg/query-service/tests/integration/filter_suggestions_test.go is well-structured, but it could benefit from additional comments explaining the purpose of each section for future maintainers.
3. frontend/src/container/TraceDetail/index.tsx:295
  • Draft comment:
    Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is not relevant to the changes made in the diff. The inline style on the 'Sider' component was not introduced or modified in this diff. The comment does not address a change made in this diff, so it should be removed.
    I might be missing the context of whether the inline style was a pre-existing issue that should be addressed regardless of the diff. However, the task is to focus on changes made in the diff.
    The task is to focus on changes made in the diff, and since the inline style was not changed, the comment is not relevant.
    The comment is not about a change made in this diff, so it should be removed.

Workflow ID: wflow_XzYAaPkAlCXleJ3e


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

Copy link

github-actions bot commented Oct 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant