From bc589db9a2783d96cf2136ef829a0fd92d976646 Mon Sep 17 00:00:00 2001 From: whereareiam Date: Sun, 4 Oct 2026 09:56:47 +0200 Subject: [PATCH] Pull request: Match GitHub's reaction picker, merge box and merge button states Measured against GitHub signed in: - The reaction picker is two rows of four 32px buttons instead of an uneven three-column grid. - A mergeable pull request gets the green box border back, a blocked one a red border, and section dividers use the muted border colour. - While the merge form is open it replaces the other sections, in a neutral box with monospace fields. - Sidebar headings turn blue on hover without an underline. Forgejo marks the collapsed merge button red whenever checks are unmet, even when the action only schedules the merge for when checks succeed. The button now follows the action: primary for a scheduled merge, danger only for an override. Co-Authored-By: Claude Opus 5.5 --- public/assets/js/pull-review.js | 9 ++++ .../components/issues/issue_conversation.ts | 25 +++++++++++ styles/components/issues/pull_review.ts | 41 +++++++++++++++++-- 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/public/assets/js/pull-review.js b/public/assets/js/pull-review.js index 860a4c7..94404b0 100644 --- a/public/assets/js/pull-review.js +++ b/public/assets/js/pull-review.js @@ -27,6 +27,15 @@ actions.append(form); if (instructions) actions.append(instructions); section.append(actions); + // Forgejo colours the collapsed button red whenever checks are unmet, even when the action + // only schedules the merge. A label longer than the plain merge action is a scheduled merge. + const plain = (state?.mergeStyles ?? []).map(style => style.textDoMerge?.trim()); + const sync = () => { + const label = form.querySelector(".merge-button .button-text")?.textContent.replace(/\s+/g, " ").trim(); + actions.classList.toggle("merge-scheduled", Boolean(label) && !plain.includes(label)); + }; + sync(); + new MutationObserver(sync).observe(form, { childList: true, subtree: true, characterData: true }); }; if (document.readyState === "loading") document.addEventListener("DOMContentLoaded", decorate, { once: true }); else decorate(); diff --git a/styles/components/issues/issue_conversation.ts b/styles/components/issues/issue_conversation.ts index 5ba02d3..5e96000 100644 --- a/styles/components/issues/issue_conversation.ts +++ b/styles/components/issues/issue_conversation.ts @@ -114,6 +114,7 @@ export default css` & .ui.dropdown > :is(a, span).text:hover { color: ${themeVars.github.fgColor.accent}; background: transparent; + text-decoration: none; } & .ui.dropdown > :is(a, span).text:hover strong { color: inherit; @@ -600,6 +601,30 @@ export default css` left: 0 !important; right: auto !important; } + /* GitHub's picker: two rows of four 32px buttons, filled column by column. */ + & .ui.dropdown.select-reaction > .menu.reactions-menu.visible { + display: grid !important; + grid-template-rows: repeat(2, 40px); + grid-auto-flow: column; + grid-auto-columns: 36px; + width: max-content; + min-width: 0; + padding: 0 2px; + border-radius: ${otherThemeVars.border.radius}; + & > .item.reaction { + display: flex; + align-items: center; + justify-content: center; + float: none; + width: 32px; + height: 32px; + margin: 4px 2px !important; + padding: 4px !important; + border-radius: ${otherThemeVars.border.radius}; + font-size: 14px; + line-height: 1; + } + } & .select-reaction > .add-reaction { display: flex; width: 28px; diff --git a/styles/components/issues/pull_review.ts b/styles/components/issues/pull_review.ts index 1c449ce..cf87639 100644 --- a/styles/components/issues/pull_review.ts +++ b/styles/components/issues/pull_review.ts @@ -1,4 +1,5 @@ import { css, otherThemeVars, themeVars } from "@lutinglt/gitea-github-theme/core"; +import { primaryButtonHoverStyle, primaryButtonStyle } from "@lutinglt/gitea-github-theme/styles/common"; /** The 32px round status icon that starts a merge box section; the section sets its background. */ const mergeSectionIcon = ` @@ -216,6 +217,25 @@ export default css` &.merge-ready > .timeline-avatar { background: ${themeVars.github.bgColor.success.emphasis}; } + &.merge-ready > .content { + border-color: ${themeVars.github.bgColor.success.emphasis}; + } + /* A blocked pull request: the box border follows the red status badge. */ + &:has(> .timeline-avatar.red) > .content { + border-color: ${themeVars.github.bgColor.danger.emphasis}; + } + /* While the merge form is open GitHub shows the form alone, in a neutral box. */ + & > .content:has(.pull-merge-actions form .field) { + border-color: ${themeVars.color.light.border}; + & > :not(.merge-section), + & > .merge-section > :not(.pull-merge-actions) { + display: none !important; + } + & .pull-merge-actions { + border-top: 0 !important; + border-radius: ${otherThemeVars.border.radius}; + } + } /* Every state uses one layout: each row is a bordered section, as in GitHub's merge box. */ & .merge-section { padding: 0; @@ -224,7 +244,7 @@ export default css` display: none; } & > :is(.item, details, .pull-merge-actions):not(:first-child) { - border-top: 1px solid ${themeVars.color.light.border}; + border-top: 1px solid color-mix(in srgb, ${themeVars.color.light.border} 70%, transparent); } & > .item { display: flex; @@ -344,6 +364,17 @@ export default css` border-color: ${themeVars.github.button.danger.borderColor.hover}; } } + /* Scheduling a merge for when checks pass is not an override: Forgejo marks the group + red, but its own confirm button is primary, so the group is shown primary too. */ + &.merge-scheduled .ui.red.buttons .ui.button { + ${primaryButtonStyle} + &:hover { + ${primaryButtonHoverStyle} + } + & svg { + color: inherit; + } + } & .ui.buttons .ui.dropdown.icon.button { width: 32px; padding: 0; @@ -386,16 +417,18 @@ export default css` flex: 0 0 100%; margin: 0; } + & > .field > :is(input, textarea) { + font-family: var(--fonts-monospace); + line-height: 20px; + } & > .field > input { height: 32px; padding: 5px 12px; - line-height: 20px; } & > .field > textarea { display: block; min-height: 100px; - padding: 8px 12px; - line-height: 20px; + padding: 12px; resize: vertical; } & > .ui.checkbox {