Skip to content

[UI] feature:builder style generalization solution#2888

Open
yueman920 wants to merge 1 commit intogoplus:uifrom
yueman920:Builder-style-generalization-solution
Open

[UI] feature:builder style generalization solution#2888
yueman920 wants to merge 1 commit intogoplus:uifrom
yueman920:Builder-style-generalization-solution

Conversation

@yueman920
Copy link

Background

-The builder needs to appeal to users of all ages and requires style generalization.

Modifications

  • Added style changes to the Sprite Editor and Generalized Community pages.

  • Added card style organization.

Scope of Impact

  • Sprite Editor page in SPX

  • AI canvas

Does it affect the design system?

  • No

@gemini-code-assist
Copy link

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@xgopilot
Copy link
Contributor

xgopilot bot commented Feb 28, 2026

This PR adds design specification assets only — no runtime code is changed. The designs themselves look well-structured, but there are several repository hygiene and documentation issues worth addressing before merging.

Key concerns:

  • The demo HTML and description .md are missing (required by ui/README.md and the PR template).
  • Both .json and .pen files are committed — they are structurally identical; only one is needed.
  • Large PNGs (17 MB+) should be tracked with Git LFS to avoid bloating every clone.
  • The "Does not affect the design system?" claim is unsubstantiated for a style-generalization PR.
  • Scope of Impact omits Community pages, which are covered by the mockups and mentioned in Modifications.

@xgopilot
Copy link
Contributor

xgopilot bot commented Feb 28, 2026

ui/features/spx/builder-style-generalization-solution_2026.02.28/main-change plan.json + .pen — Redundant dual-format storage

Both files are Penpot JSON exports and are structurally identical, differing by only ~2 lines (image path references). Committing both adds ~4.78 MB of duplicate data. The same pattern exists in all sibling feature directories. Please commit only one format (.pen is the canonical Penpot format) and drop the redundant .json, or document which is authoritative in ui/README.md.

@xgopilot
Copy link
Contributor

xgopilot bot commented Feb 28, 2026

ui/features/spx/images/ (32 PNGs, ~17 MB) — Missing Git LFS tracking

There is no .gitattributes or LFS configuration in this repository. All PNG mockups and the large JSON design files are stored as regular git blobs, adding ~22 MB to every clone and CI checkout unconditionally. Please configure Git LFS for *.png, *.pen, and large *.json design files under ui/features/:

# .gitattributes
ui/features/**/*.png filter=lfs diff=lfs merge=lfs -text
ui/features/**/*.pen filter=lfs diff=lfs merge=lfs -text

This is especially important as the ui/features/ directory grows with each new design spec PR.

@xgopilot
Copy link
Contributor

xgopilot bot commented Feb 28, 2026

Missing required artefacts per ui/README.md

The feature directory builder-style-generalization-solution_2026.02.28/ is missing two items that ui/README.md and the PR template list as mandatory:

  1. Demo HTML — an interactive prototype (see Change-account-name-and-ID_2026.02.13/modal-account-name_demo.html as a reference)
  2. Description .md — a design explanation document (see Change-account-name-and-ID_2026.02.13/Figma-design-file-link.md)

Please add these before merging so the feature directory is complete per the established convention.

@xgopilot
Copy link
Contributor

xgopilot bot commented Feb 28, 2026

PR description: Scope of Impact is incomplete + "Does not affect design system" is unsubstantiated

Two issues with the PR description:

  1. Scope of Impact lists "Sprite Editor page in SPX, AI canvas" but the Modifications section itself says "Generalized Community pages" — and the mockup images confirm this (e.g., figma-community.png, figma-private.png, figma-team.png). Please add Community pages to the Scope.

  2. "Does it affect the design system? No" — A PR titled "style generalization solution" that introduces new visual tokens across multiple pages makes this claim hard to accept without supporting evidence. Please add a brief justification (e.g., "all tokens used are already defined in the design system") or update to "Yes" if new tokens are introduced.

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