Skip to content

fix: render all interval tabs for visual consistency - #7969

Open
lavjeetrai wants to merge 2 commits into
sugarlabs:masterfrom
lavjeetrai:fix-interval-tabs-clean
Open

fix: render all interval tabs for visual consistency#7969
lavjeetrai wants to merge 2 commits into
sugarlabs:masterfrom
lavjeetrai:fix-interval-tabs-clean

Conversation

@lavjeetrai

Copy link
Copy Markdown
Contributor

This PR fixes a UI consistency issue in the pie menus by ensuring all tabs are always rendered for each interval type.

Previously, inactive tabs were completely hidden from the DOM, which could disrupt the structural layout of the pie menu slices and cause unexpected shifting. This resolves the TODO: Add all tabs to each interval by always showing all tabs and instead setting .enabled = false for the inactive ones. This keeps the pie menu's layout and slice positioning consistent across intervals.

Changes Made

  • Modified __setupAction in js/piemenus.js to remove the .hide() logic for inactive tabs.

  • Set inactive tabs to .enabled = false so that they stay in the DOM but are correctly marked as disabled.

  • Bug Fix

@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior size/XS Extra small: < 10 lines changed area/javascript Changes to JS source files labels Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.16%. Comparing base (748bca6) to head (3d92d31).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7969      +/-   ##
==========================================
+ Coverage   61.97%   62.16%   +0.18%     
==========================================
  Files         187      187              
  Lines       58368    58367       -1     
==========================================
+ Hits        36176    36283     +107     
+ Misses      22192    22084     -108     

☔ View full report in Codecov by Harness.
📢 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.

@github-actions github-actions Bot added size/M Medium: 50-249 lines changed area/tests Changes to test files and removed size/XS Extra small: < 10 lines changed labels Aug 5, 2026
@lavjeetrai

Copy link
Copy Markdown
Contributor Author

@walterbender could you please review this pr when u got time.

@lavjeetrai

Copy link
Copy Markdown
Contributor Author

@Ashutoshx7 could u take a look to this

@walterbender

Copy link
Copy Markdown
Member

@abhnish can you look at this?
@lavjeetrai can you include a before/after screenshot so I know what to test?

@abhnish

abhnish commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@lavjeetrai the layout-shift issue makes sense . Before I approve, a couple of things, does enabled = false actually disable interaction, or is it just a flag right now? Since tabs stay in the DOM instead of being removed, I want to confirm inactive tabs aren't still keyboard/screen-reader focusable. Also, the test only covers the "perfect" interval, so it'd be good to add a case for "minor" too, along with a test asserting inactive tabs are no longer hidden. One more thing - in __setupAction, tabs from other interval types (l !== i) are still .hide()d, only the active interval's own tabs use the new enabled approach, so the PR description saying "all tabs" reads as global when it's actually scoped to the active interval; might be worth clarifying that.
Otherwise looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior size/M Medium: 50-249 lines changed

Projects

Development

Successfully merging this pull request may close these issues.

3 participants