Skip to content

fix(core): prevent slide toolbar title overlap on mobile#241

Open
ridemountainpig wants to merge 1 commit into
1weiho:mainfrom
ridemountainpig:fix/mobile-toolbar-title-overlap
Open

fix(core): prevent slide toolbar title overlap on mobile#241
ridemountainpig wants to merge 1 commit into
1weiho:mainfrom
ridemountainpig:fix/mobile-toolbar-title-overlap

Conversation

@ridemountainpig

@ridemountainpig ridemountainpig commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

What

Fixes the slide toolbar on narrow screens:

  • Title no longer overlaps the toolbar icons. On mobile the two side groups each take flex-1, so the in-flow title lands at the viewport center, and min-w-0 lets it truncate instead of overlapping. On md+ the title keeps its original absolute centering.
  • Copy-link and download actions collapse into a single overflow menu (MoreHorizontal) on mobile. On md+ the standalone copy-link button and download dropdown render as before.

The export menu items were extracted into a shared exportMenuItems fragment reused by both the desktop download dropdown and the mobile overflow menu, removing duplicated handler code.

Before:
Dia 2026-06-15 20 59 02

After:
Dia 2026-06-15 20 58 57
Dia 2026-06-15 20 59 11

Notes

  • Adds a moreActions locale string across en / ja / zh-cn / zh-tw and the Locale type.
  • Changeset included (@open-slide/core patch).
  • tsc and Biome pass clean on the changed files.

🤖 Generated with Claude Code

Keep the title from overlapping the toolbar icons on narrow screens, and
collapse the copy-link and download actions into a single overflow menu.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

@ridemountainpig is attempting to deploy a commit to the Yiwei Ho Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The slide toolbar is refactored to fix title/icon overlap on narrow screens. Copy-link and export operations are extracted into shared async handlers. A new mobile overflow dropdown consolidates these actions, and the title wrapper gains responsive positioning. A moreActions locale key is added across all four supported languages.

Changes

Mobile Toolbar Overflow Menu & Responsive Layout Fix

Layer / File(s) Summary
moreActions locale type and translations
packages/core/src/locale/types.ts, packages/core/src/locale/en.ts, packages/core/src/locale/ja.ts, packages/core/src/locale/zh-cn.ts, packages/core/src/locale/zh-tw.ts
Adds moreActions: string to the Locale type and populates translations in English, Japanese, Simplified Chinese, and Traditional Chinese.
Shared async toolbar action handlers
packages/core/src/app/routes/slide.tsx
Extracts copyLink, exportHtml, exportPdf, and exportImagePptx async handlers with exporting/linkCopied state, toast lifecycle management, and a reusable exportMenuItems fragment; adds MoreHorizontal icon import.
Responsive toolbar layout and mobile overflow menu
packages/core/src/app/routes/slide.tsx
Updates the toolbar title wrapper to a responsive flex/absolute layout, hides the desktop copy-link button on mobile (hidden md:inline-flex), and replaces the mobile export inline menu with a more-actions overflow dropdown wired to the shared handlers.
Changeset entry
.changeset/fix-mobile-title-overlap.md
Documents the @open-slide/core patch for the mobile title overlap fix and overflow menu consolidation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • 1weiho/open-slide#188: Modifies the same slide.tsx export dropdown/menu wiring, overlapping with the image-PPTX/PPTX export handler changes in this PR.
  • 1weiho/open-slide#154: Also changes the centered title/header layout in slide.tsx to prevent overlap with right-side controls on narrow viewports.
  • 1weiho/open-slide#132: Implements the original copy-link button and related locale/type additions that this PR refactors into a shared handler.

Suggested reviewers

  • 1weiho

Poem

🐰 On tiny screens, no icons shall clash,
The title stays put, not lost in a flash.
A "more actions" menu hops into view,
Copy and export all neatly queued through.
The rabbit tidied the toolbar with care —
Now mobile and desktop both get their fair share! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: preventing slide toolbar title overlap on mobile screens, which is the primary fix addressed across all modified files.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/core/src/app/routes/slide.tsx (1)

541-548: 💤 Low value

Consider removing the layout comment per coding guidelines.

The comment at lines 541-543 explains the responsive centering strategy. While helpful, the coding guidelines specify "Default to writing no comments" for TypeScript files. The layout behavior is somewhat self-documenting through the class names (flex-1, min-w-0, md:absolute), though the centering mechanism is subtle. Consider whether the comment adds enough non-obvious insight to justify the exception.

As per coding guidelines, "Default to writing no comments. Only add one when the WHY is non-obvious — a hidden constraint, a subtle invariant, a workaround for a specific bug, or behavior that would surprise a reader."

🤖 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 `@packages/core/src/app/routes/slide.tsx` around lines 541 - 548, Evaluate and
potentially remove the multi-line comment at the InlineTitleEditor section
(lines 541-543) explaining the responsive centering strategy. According to
coding guidelines, comments should only be retained when they explain
non-obvious WHY decisions—not HOW the code works. Since the Tailwind classes
(flex-1, min-w-0, md:absolute, md:inset-x-0) are relatively self-documenting of
the layout behavior, remove the comment unless you determine it documents a
non-obvious constraint or hidden invariant that would surprise readers.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@packages/core/src/app/routes/slide.tsx`:
- Around line 541-548: Evaluate and potentially remove the multi-line comment at
the InlineTitleEditor section (lines 541-543) explaining the responsive
centering strategy. According to coding guidelines, comments should only be
retained when they explain non-obvious WHY decisions—not HOW the code works.
Since the Tailwind classes (flex-1, min-w-0, md:absolute, md:inset-x-0) are
relatively self-documenting of the layout behavior, remove the comment unless
you determine it documents a non-obvious constraint or hidden invariant that
would surprise readers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4717835b-d5af-4948-937c-f95081ecc071

📥 Commits

Reviewing files that changed from the base of the PR and between 0128ba4 and 50c7176.

📒 Files selected for processing (7)
  • .changeset/fix-mobile-title-overlap.md
  • packages/core/src/app/routes/slide.tsx
  • packages/core/src/locale/en.ts
  • packages/core/src/locale/ja.ts
  • packages/core/src/locale/types.ts
  • packages/core/src/locale/zh-cn.ts
  • packages/core/src/locale/zh-tw.ts

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