Skip to content

Conversation

Jitmisra
Copy link
Contributor

redesigned node page
Monosnap Apache Kvrocks Controller
Monosnap Apache Kvrocks Controller 2025-07-24 11-35-41

@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.08%. Comparing base (6c56470) to head (80b7a12).
Report is 75 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #327      +/-   ##
============================================
+ Coverage     43.38%   47.08%   +3.69%     
============================================
  Files            37       45       +8     
  Lines          2971     4420    +1449     
============================================
+ Hits           1289     2081     +792     
- Misses         1544     2130     +586     
- Partials        138      209      +71     
Flag Coverage Δ
unittests 47.08% <ø> (+3.69%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@PragmaTwice PragmaTwice requested a review from Copilot July 24, 2025 12:25
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR redesigns the node page in the webui with a complete UI overhaul, introducing a responsive sidebar, comprehensive search/filter functionality, and a modernized card-based layout for displaying nodes. The changes significantly enhance the user experience by adding mobile responsiveness and improved visual hierarchy.

  • Added responsive sidebar with mobile toggle functionality and adaptive width
  • Implemented search, filter, and sort controls for nodes with comprehensive filter/sort popovers
  • Redesigned node display from grid cards to a streamlined list-based layout with enhanced visual elements

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
webui/src/app/ui/sidebar.tsx Added responsive sidebar with mobile detection, toggle functionality, and adaptive width controls
webui/src/app/ui/formDialog.tsx Updated dialog typography from subtitle1 to h6 and enhanced input styling with increased height
webui/src/app/ui/formCreation.tsx Added optional children prop to NodeCreation component for custom trigger elements
webui/src/app/namespaces/[namespace]/clusters/[cluster]/shards/[shard]/page.tsx Complete page redesign with search/filter/sort functionality and modernized node display layout
Comments suppressed due to low confidence (4)

webui/src/app/namespaces/[namespace]/clusters/[cluster]/shards/[shard]/page.tsx:143

  • The sort option 'uptime-desc' is misleading as it actually sorts by creation time (newest first), not uptime. Consider renaming to 'created-desc' or 'newest' for clarity.
                case "uptime-desc":

webui/src/app/namespaces/[namespace]/clusters/[cluster]/shards/[shard]/page.tsx:145

  • The sort option 'uptime-asc' is misleading as it actually sorts by creation time (oldest first), not uptime. Consider renaming to 'created-asc' or 'oldest' for clarity.
                case "uptime-asc":

webui/src/app/namespaces/[namespace]/clusters/[cluster]/shards/[shard]/page.tsx:465

  • The label 'Newest' corresponds to 'uptime-desc' sort option, but the actual sorting is by creation time. This creates confusion between uptime and creation time concepts.
                                                        Newest

webui/src/app/namespaces/[namespace]/clusters/[cluster]/shards/[shard]/page.tsx:489

  • The label 'Oldest' corresponds to 'uptime-asc' sort option, but the actual sorting is by creation time. This creates confusion between uptime and creation time concepts.
                                                        Oldest

Comment on lines +125 to +127
if (!`node ${idx + 1}`.toLowerCase().includes(searchTerm.toLowerCase())) {
return false;
}
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

The search logic uses a hardcoded string 'node ${idx + 1}' which creates a coupling between the search functionality and the display label. Consider extracting this to a utility function or using actual node properties for searching.

Suggested change
if (!`node ${idx + 1}`.toLowerCase().includes(searchTerm.toLowerCase())) {
return false;
}
const displayLabel = generateNodeLabel(idx);
if (!displayLabel.toLowerCase().includes(searchTerm.toLowerCase())) {
return false;

Copilot uses AI. Check for mistakes.

@PragmaTwice PragmaTwice merged commit 99cea5c into apache:unstable Jul 24, 2025
4 checks passed
@PragmaTwice
Copy link
Member

Thank you for your contribution!

@Jitmisra Jitmisra deleted the redesign-node branch July 30, 2025 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants