Skip to content

Only hide infopanel on low-res landscape#697

Merged
lawsie merged 3 commits into
flipcomputing:mainfrom
lawsie:bugfix-shortcuts-mobile
Jul 9, 2026
Merged

Only hide infopanel on low-res landscape#697
lawsie merged 3 commits into
flipcomputing:mainfrom
lawsie:bugfix-shortcuts-mobile

Conversation

@lawsie

@lawsie lawsie commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #693 hid the info panel (containing the shortcuts docs and the Flock link) at narrow resolutions as there wasn't enough room to display the info panel. This unintentionally placed the Flock logo over the top of the pill toggle on mobile.

This PR only hides the infopanel on low resolution (<1024px width) LANDSCAPE screens where there is no room for it. On narrow portrait screens (i.e. mobile phones in portrait) there is plenty of room so the infopanel bar is displayed.

AI usage

Claude Sonnet 4.6 and Fable 5 (ooo) used throughout, decisions made by a human.

Summary by CodeRabbit

  • Bug Fixes

    • Improved responsive behavior for the shortcuts and info panels on narrow screens, especially in landscape mode.
    • Fixed area navigation so the correct control is targeted when the info panel tabs are not visible.
    • Restored focus correctly when switching the shortcuts panel back from modal to docked mode.
    • Adjusted layout spacing so the fixed bottom bar no longer overlaps docked panel controls.
  • Tests

    • Updated and expanded coverage for responsive panel switching, visibility-based navigation, and focus restoration.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refines narrow-layout detection for the shortcuts panel to require landscape orientation, remaps area 5 based on #info-panel-tabs visibility, restores focus when exiting modal mode on resize, updates responsive CSS for the bottom bar and resizer, and adjusts related tests.

Changes

Accessibility and layout fixes

Layer / File(s) Summary
Narrow layout and area 5 remapping
accessibility/keyboardui.js
isNarrowLayout() now requires max-width: 1024px and orientation: landscape; area 5 remaps to #viewToggle based on #info-panel-tabs visibility (offsetWidth === 0 or missing) rather than layout breakpoint alone.
Focus restoration on modal exit
accessibility/keyboardui.js
Resize handler transitioning from narrow-modal to docked now calls exitModal() and restores focus to previousFocus.
Responsive CSS and resizer visibility
style.css, main/view.js
Adds narrow-layout spacing for the bottom bar, hides the resizer at narrow widths, hides the bottombar flocklink by default, adjusts the logo height in narrow landscape mode, and clears inline resizer display overrides so stylesheet rules apply.
Test coverage for area remapping and modal resize
tests/keyboardui.test.js
AreaManager tests assert DOM-driven area 5 mapping based on #info-panel-tabs presence/visibility; a new resize-triggered modal switching suite verifies modal entry/exit and focus restoration.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

  • flipcomputing/flock#595: Both PRs are part of the keyboard shortcuts panel work and touch accessibility/keyboardui.js around modal/docked behavior and focus handling.
  • flipcomputing/flock#643: Both PRs modify shortcuts panel behavior in accessibility/keyboardui.js, including resize- and layout-driven state changes.
  • flipcomputing/flock#693: Both PRs update accessibility/keyboardui.js to remap Area 5 in narrow layouts and switch ShortcutsPanel modal state on resize, including focus behavior.

Suggested reviewers: tracygardner

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: hiding the infopanel only in low-resolution landscape layouts.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
style.css (1)

1019-1021: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Magic number 85px in #blocklyDiv height calculation.

The #bottomBar height is 52px (line 939) and #info-panel-tabs margin-bottom is 35px (line 1016), totaling 87px — but #blocklyDiv subtracts 85px. If this offset is meant to account for both elements, it's off by 2px. If it accounts for something else, the relationship should be documented or derived from the same values to avoid drift.

♻️ Suggested refactor: derive from shared values
-  `#blocklyDiv` {
-    height: calc(100% - 85px - max(0px, env(safe-area-inset-bottom, 0px))) !important;
-  }
+  `#blocklyDiv` {
+    height: calc(100% - 52px - 35px - max(0px, env(safe-area-inset-bottom, 0px))) !important;
+  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@style.css` around lines 1019 - 1021, The `#blocklyDiv` height calculation uses
a hardcoded 85px offset that appears to drift from the related layout values.
Update the `#blocklyDiv` rule to derive its subtraction from the same shared size
values used by `#bottomBar` and `#info-panel-tabs`, or adjust it so the total
matches those symbols consistently; keep the relationship centralized to avoid
future mismatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@style.css`:
- Around line 1032-1046: The landscape responsive rules need to hide the drag
handle at the 769–1024px breakpoint as well, since `.resizer` can remain visible
when `aside#info-panel` is hidden. Update the media query in the
`#info-panel`/`.resizer` responsive block so the `.resizer` selector is also
covered in the landscape ≤1024px case, and remove the redundant `#info-panel {
display: none; }` declaration because `aside#info-panel` already handles that
state.

---

Nitpick comments:
In `@style.css`:
- Around line 1019-1021: The `#blocklyDiv` height calculation uses a hardcoded
85px offset that appears to drift from the related layout values. Update the
`#blocklyDiv` rule to derive its subtraction from the same shared size values used
by `#bottomBar` and `#info-panel-tabs`, or adjust it so the total matches those
symbols consistently; keep the relationship centralized to avoid future
mismatch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 203e9d11-07f5-428d-8227-e6949e56c3b5

📥 Commits

Reviewing files that changed from the base of the PR and between ab407c7 and 07d38b5.

📒 Files selected for processing (3)
  • accessibility/keyboardui.js
  • style.css
  • tests/keyboardui.test.js

Comment thread style.css Outdated
@lawsie lawsie merged commit c646061 into flipcomputing:main Jul 9, 2026
2 of 4 checks passed
@lawsie lawsie deleted the bugfix-shortcuts-mobile branch July 9, 2026 12:41
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.

1 participant