Skip to content

fix: center Run Query button text with flexbox#86

Merged
cyyeh merged 1 commit into
mainfrom
fix/run-query-btn-center-text
Mar 2, 2026
Merged

fix: center Run Query button text with flexbox#86
cyyeh merged 1 commit into
mainfrom
fix/run-query-btn-center-text

Conversation

@cyyeh
Copy link
Copy Markdown
Owner

@cyyeh cyyeh commented Mar 2, 2026

Summary

  • Replace text-align: center + padding: 8px 20px with flexbox centering on the Run Query button
  • Ensures text is perfectly centered regardless of i18n language

Test plan

  • Verify Run Query button text is centered in English
  • Verify Run Query button text is centered in zh-TW

🤖 Generated with Claude Code

Replace text-align/padding centering with flexbox to ensure text is
perfectly centered both horizontally and vertically.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 4626b09d9fd3
@cyyeh cyyeh merged commit 2368ced into main Mar 2, 2026
2 checks passed
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Mar 2, 2026

Claude finished @cyyeh's task in 1m 1s —— View job


Code Review: fix: center Run Query button text with flexbox

Tasks

  • Gather context
  • Review changed files
  • Post review feedback

Overview

This is a small, focused CSS fix that replaces text-align: center + padding: 8px 20px on the Run Query button with a flexbox-based centering approach. The motivation — ensuring proper centering for i18n text — is sound.


What Changed (frontend/src/components/QueryEditor.css)

Before (inferred from diff):

.query-editor__run-btn {
  width: 100px;
  height: 38px;
  text-align: center;
  padding: 8px 20px;
  ...
}

After:

.query-editor__run-btn {
  width: 100px;
  height: 38px;
  padding: 0;
  ...
  box-sizing: border-box;
  display: flex;
  align-items: center;
  justify-content: center;
  white-space: nowrap;
}

Strengths

  • Correct approach: display: flex + align-items: center + justify-content: center is the modern, reliable way to center button content both horizontally and vertically. text-align: center only addresses horizontal alignment.
  • Fixes the root cause: text-align on a <button> can be inconsistent across browsers and doesn't guarantee vertical centering, especially when button height is explicitly set. Flexbox eliminates that ambiguity.
  • white-space: nowrap: Good addition — prevents text from wrapping inside a fixed-width button.
  • padding: 0: Correct to zero out padding when using flexbox with a fixed height: 38px, since centering is handled by flex alignment rather than padding.
  • Cross-browser compatibility: Flexbox has >96% global browser support and works correctly on all modern browsers for <button> elements.

Observations

1. Fixed width may still clip long i18n strings

The stated motivation is correct centering across i18n languages, but the fixed width: 100px combined with white-space: nowrap means text that exceeds 100px will overflow the button visually rather than push the button wider. For zh-TW this likely works fine, but languages with longer translations could be affected.

Consider replacing width with min-width for better long-term i18n robustness:

min-width: 100px;
/* width: 100px; */

Fix this →

2. box-sizing: border-box is redundant with padding: 0

box-sizing: border-box only makes a difference when the element has padding or border that would otherwise expand its dimensions. Since padding: 0 means there's no padding to account for, box-sizing: border-box has no effect here. It's not harmful, but it's technically unnecessary. If a global *, *::before, *::after { box-sizing: border-box; } reset is already applied (common in React projects), it's doubly redundant.


Summary

This is a clean, correct, and well-motivated fix. The flexbox approach is strictly better than text-align: center for this use case. The only actionable suggestion is considering min-width instead of width to future-proof against i18n languages with longer button labels.

Verdict: Approved — minor improvement opportunity noted above.

@cyyeh cyyeh deleted the fix/run-query-btn-center-text branch March 2, 2026 09:21
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