[Page] Update: Adjust editor and tutorials layout#3192
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables the license display in the environment settings, updates the editor's background to use a shared UI token, and adjusts the layout of the course series page. Feedback suggests using flex-grow flex-shrink-0 instead of flex-1 on the CenteredWrapper to avoid potential scrolling issues in fixed-height containers. Additionally, the TODO comment in the editor page regarding the background color should be removed as it is now obsolete.
| <CommunityNavbar /> | ||
|
|
||
| <CenteredWrapper size="medium" class="my-6"> | ||
| <CenteredWrapper size="medium" class="my-6 flex-1"> |
There was a problem hiding this comment.
Using flex-1 (which sets flex-basis: 0%) on the CenteredWrapper while the parent container (line 100) has a fixed height (h-full) and overflow-y-auto will prevent the page from scrolling when the content is long. In a flex container with a fixed height, flex-1 constrains the element to the remaining space. Since the total height of the children will then exactly match the parent's height, the parent's scrollbar will not be triggered, potentially cutting off content.
To allow the content to grow naturally and trigger the parent's scrollbar while still pushing the footer to the bottom when the content is short, consider using flex-grow with flex-shrink-0 and an auto basis.
<CenteredWrapper size="medium" class="my-6 flex-grow flex-shrink-0">
| <!-- TODO: review root background color and promote it to a shared UI token if it becomes reused. --> | ||
| <template> | ||
| <section class="min-h-full w-full flex flex-col bg-[#f1f5f7]"> | ||
| <section class="min-h-full w-full flex flex-col bg-grey-300"> |
There was a problem hiding this comment.
The background color has been updated to bg-grey-300. Note that the TODO comment at line 1 is now obsolete since the color is being reused as a shared token across the application (e.g., in the tutorials page), fulfilling the condition mentioned in the comment. It should be removed to keep the code clean.
qingqing-ux
force-pushed
the
editor-background-color
branch
from
63e7f3d to
a3df993
Compare
| @@ -1,6 +1,6 @@ | |||
| <!-- TODO: review root background color and promote it to a shared UI token if it becomes reused. --> | |||
There was a problem hiding this comment.
已处理：删除了这行过期 TODO。
|
|
||
| # Whether to show the license information (including copyright) in the footer. | ||
| VITE_SHOW_LICENSE="false" | ||
| VITE_SHOW_LICENSE="true" |
There was a problem hiding this comment.
已处理：已将默认配置恢复为 VITE_SHOW_LICENSE="false"。
| <CommunityNavbar /> | ||
|
|
||
| <CenteredWrapper size="medium" class="my-6"> | ||
| <CenteredWrapper size="medium" class="my-6 flex-1"> |
There was a problem hiding this comment.
gemini 说的有点道理，虽然实测没发现问题，还是帮忙改成 flex-[1_0_auto] 吧
There was a problem hiding this comment.
已处理：已按建议改为 flex-[1_0_auto]，避免 flex-basis: 0% 带来的滚动风险。
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Small, focused diff. Layout intent is clear and the changes are generally correct. A couple of things worth addressing:
- No security issues found.
- No performance issues found (replacing an arbitrary color value with a design token is a net bundle improvement).
|
|
||
| # Whether to show the license information (including copyright) in the footer. | ||
| VITE_SHOW_LICENSE="false" | ||
| VITE_SHOW_LICENSE="true" |
There was a problem hiding this comment.
Changing the base-default to "true" means every environment that doesn't explicitly override this flag (local development, staging, non-China-facing deployments) will now render the Chinese regulatory footer. The previous "false" was a safe opt-out default.
The footer content (CommunityFooter) displays ICP and public-network-security filing links that are only legally relevant to the China-facing production deployment. Consider keeping the base default as "false" and setting VITE_SHOW_LICENSE="true" in the specific .env.[mode] file(s) targeting that deployment — consistent with the opt-in pattern already used for VITE_SHOW_TUTORIALS_ENTRY in .env.staging.
| @@ -1,6 +1,6 @@ | |||
| <!-- TODO: review root background color and promote it to a shared UI token if it becomes reused. --> | |||
There was a problem hiding this comment.
The TODO condition — "if it becomes reused" — is now met. bg-grey-300 is used as the page-root background in at least this file, tutorials/course-series.vue, tutorials/index.vue, and community/index.vue. The comment reads as a future conditional but the reuse has already happened.
Consider either promoting the pattern to a documented layout token / shared CSS class, or removing the TODO if the current design-token approach (bg-grey-300) is considered sufficient.
| <CommunityNavbar /> | ||
|
|
||
| <CenteredWrapper size="medium" class="my-6"> | ||
| <CenteredWrapper size="medium" class="my-6 flex-1"> |
There was a problem hiding this comment.
Adding flex-1 to push CommunityFooter to the bottom is the right approach. Worth a quick visual check when the course list is short (e.g. 1–2 courses) to confirm the footer lands at the bottom of the viewport rather than floating mid-page, since CenteredWrapper internally applies self-center which can interact with flex growth in some edge cases.
2 participants
Issue
Background
Align the editor and tutorials pages with the expected UI behavior: use the shared grey background token, keep the course-series content area filling the remaining page height, and show the required license information in local/default configuration.
Changes
bg-grey-300.flex-1so the footer can sit at the bottom of the page.VITE_SHOW_LICENSE.Scope
Design System Impact