Skip to content

Yan fix piechart visualization#4928

Merged
one-community merged 18 commits into
developmentfrom
yanyan_fix_piechart_visualization_4031
May 14, 2026
Merged

Yan fix piechart visualization#4928
one-community merged 18 commits into
developmentfrom
yanyan_fix_piechart_visualization_4031

Conversation

@yany960
Copy link
Copy Markdown
Contributor

@yany960 yany960 commented Mar 2, 2026

Description

image

Video: https://www.loom.com/share/c177e2f6c87c4005aad39dff9b99460f?sid=1a103e3c-dc92-4e62-9c2f-3b8c52f5305e
image

Video: https://drive.google.com/file/d/1Yl4CFf2yjRINe8chVIkw98kZsPPvFvWv/view

Fixes # (Priority: Medium)

Related PRS (if any):

#4031

Main changes explained:

  • Delete file A for removing unused components …
  • Update file B for including new pattern …
  • Create file C for introducing new components …

How to test:

  1. Check into current branch
  2. Do npm install and ... to run this PR locally
  3. Clear site data/cache
  4. Go to Dashboard → Reports → Reports → Projects
  5. Select any Project name and Toggle the Checkboxes to view PieCharts (Example: /projectreport/6689dbd6c5e25463a05bcc9e)
  6. Verify the changes made (In Both Dark Mode and Light Mode)

Screenshots or videos of changes:

image

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 2, 2026

Deploy Preview for highestgoodnetwork-dev ready!

Name Link
🔨 Latest commit bbc5285
🔍 Latest deploy log https://app.netlify.com/projects/highestgoodnetwork-dev/deploys/69db3ddb25e15d000894bade
😎 Deploy Preview https://deploy-preview-4928--highestgoodnetwork-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@yany960 yany960 added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label Mar 2, 2026
rohanrastogi311
rohanrastogi311 previously approved these changes Mar 7, 2026
Copy link
Copy Markdown

@rohanrastogi311 rohanrastogi311 left a comment

Choose a reason for hiding this comment

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

Hi Yanyan,

Well done with this implementation

Image

yany960 pushed a commit that referenced this pull request Mar 9, 2026
- Add adaptive gap calculation based on screen size and data count
- Add dynamic text offset for different screen sizes
- Limit number of labels shown on small screens (6 on mobile, 8 on tablet)
- Simplify label text on smaller screens for better readability
- Ensure layout recalculates when window size changes

Fixes PR #4928 responsive text overlap issue
- Add adaptive gap calculation based on screen size and data count
- Add dynamic text offset for different screen sizes
- Limit number of labels shown on small screens (6 on mobile, 8 on tablet)
- Simplify label text on smaller screens for better readability
- Ensure layout recalculates when window size changes
yany0309 added 4 commits March 9, 2026 22:54
- Add iterative dodgeY with backward pass to prevent overlaps
- Increase base gap from 16/20 to 20/24 pixels
- Expand vertical bounds from R+60 to R+100 for more space
- Add third pass to ensure labels stay within bounds
@yany960 yany960 force-pushed the yanyan_fix_piechart_visualization_4031 branch from 604e6a2 to b95d1f8 Compare March 9, 2026 16:17
- Add automatic aggregation of small values into 'Others' category
- Implement smart label distribution from center outward
- Add adaptive gap calculation based on available space
- Improve responsive design with dynamic container heights
- Simplify text display for smaller screens
@yany960 yany960 requested a review from rohanrastogi311 March 10, 2026 07:08
@yany960 yany960 changed the title Yanyan fix piechart visualization 4031 Yan fix piechart visualization Mar 10, 2026
Copy link
Copy Markdown

@Anusha-Gali Anusha-Gali left a comment

Choose a reason for hiding this comment

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

Hi Yan,

I have reviewed your PR locally and although the functionality works as per requirement, found only one issues - the UI in mobile responsiveness is not compatible to the mobile

Image Image Image Image Image Image Image Image Image

@yany960 yany960 force-pushed the yanyan_fix_piechart_visualization_4031 branch from cf46e1b to 8654aa6 Compare March 17, 2026 05:47
- Move responsive styles from CSS to inline JSX styles
- ProjectPieChart: dynamic button-container positioning and scale based on windowSize
- PieChartByProject: inline responsive styles for title, description, labels, and text
- AGGRESSIVELY reduce pie chart size on mobile:
  - <=400px: outerRadius 90px (was 135px)
  - <=576px: outerRadius 105px (was 140px)
  - <=640px: outerRadius 120px
- Reduce container dimensions and text offset for mobile
- Reduce parent container height: 22rem/26rem/32rem based on screen size
- Ensure proper display on iPhone and other mobile devices
@yany960 yany960 force-pushed the yanyan_fix_piechart_visualization_4031 branch 5 times, most recently from ea9f9be to 587daca Compare March 17, 2026 06:29
@yany960 yany960 force-pushed the yanyan_fix_piechart_visualization_4031 branch from 587daca to 957afa6 Compare March 17, 2026 06:34
- Use inline styles in JSX instead of CSS media queries
- ProjectPieChart.jsx: dynamic circleSize, container dimensions, textOffset, fontSize
- ProjectPieChart.jsx: inline styles for button container positioning
- PieChartByProject.jsx: dynamic parent container height based on screen width
- PieChartByProject.jsx: add missing isMobile, isTablet, isSmallMobile variable definitions
- IMPROVED: Reduce pie size more aggressively on mobile (outerRadius 75px/85px)
- IMPROVED: Simplify center text to 'All' + hours only on small screens
- IMPROVED: Smaller switch button scale (0.55/0.65) on mobile
- IMPROVED: Show only hours (e.g., '43.2h') in labels on small screens
- IMPROVED: Reduce text offset further (10px/20px) to prevent cutoff
- BUGFIX: Pass windowSize parameter to renderActiveShape function
- BUGFIX: Fix button position to always center (top: 50%) instead of varying percentages
- ENHANCEMENT: Center text changes between 'All'/'Selected' based on showAllValues state
- ENHANCEMENT: Button scales down further when item selected (0.45/0.55 vs 0.55/0.65)
- Keep CSS files identical to development branch (no changes)
- Ensures mobile display on iPhone and other devices without violating CSS Module policy
@yany960 yany960 force-pushed the yanyan_fix_piechart_visualization_4031 branch from 957afa6 to 7b220e2 Compare March 17, 2026 06:43
}, [mergedProjectUsersArray,showMembers])

// Detect screen size for responsive styling
const isMobile = windowSize.width <= 576;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CSS media queries should handle responsiveness instead.

Copy link
Copy Markdown

@rohanrastogi311 rohanrastogi311 left a comment

Choose a reason for hiding this comment

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

Hi Yan,

Well done.

Image

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@Anusha-Gali Anusha-Gali left a comment

Choose a reason for hiding this comment

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

Hi Yan,

The pie chart labels are not visible in mobile responsive mode.
Issue
Image

Image Image Image

@rapolusidhartha
Copy link
Copy Markdown

Hii
The piechart visualization is perfectly working
Screenshot 2026-04-25 165607
Screenshot 2026-04-25 165621
Screenshot 2026-04-25 165640

Copy link
Copy Markdown

@rapolusidhartha rapolusidhartha left a comment

Choose a reason for hiding this comment

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

Great Job working perfect

@one-community
Copy link
Copy Markdown
Member

Thank you all, merging!

@one-community one-community merged commit 6f21716 into development May 14, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants