Skip to content

fix(ui): fix app footer layout and build environment on narrow screens - #963

Closed
userquin wants to merge 10 commits into
npmx-dev:mainfrom
userquin:fix-app-footer-layout
Closed

fix(ui): fix app footer layout and build environment on narrow screens#963
userquin wants to merge 10 commits into
npmx-dev:mainfrom
userquin:fix-app-footer-layout

Conversation

@userquin

@userquinuserquin commented Feb 4, 2026

Copy link
Copy Markdown
Member

This PR just moves the BuildEnvironment.vue after links container updating its styles to add some margins.

I need to add some test: from root run vite test --project nuxt -t BuildEnvironment

/cc @danielroe I haven't found a way to mock useAppConfig => small refactor at BuildEnvironment to accept BuildInfo and updated modules/build-env.ts to mock it (maybe using useRuntimeConfig().public instead ? ).

@vercel

vercelBot commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
docs.npmx.devErrorErrorFeb 5, 2026 2:43am
npmx.devReadyReadyPreview, CommentFeb 5, 2026 2:43am
1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
npmx-lunariaIgnoredIgnoredFeb 5, 2026 2:43am

Request Review

@coderabbitai

coderabbitaiBot commented Feb 4, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

BuildEnvironment was moved in AppFooter to a separate container while preserving the existing v-if="!isHome" condition. BuildEnvironment now accepts an optional buildInfo?: BuildInfo prop with a computed fallback to appConfig.buildInfo, and its root div class bindings were adjusted. modules/build-env.ts adds and uses the EnvType type and conditionally initialises appConfig.buildInfo with a test payload when process.env.TEST is set; otherwise it derives values via getEnv. shared/types adds EnvType and updates BuildInfo.env. Unit tests for AppFooter and BuildEnvironment were added.

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check nameStatusExplanation
Description check✅ PassedThe pull request description accurately describes the main changes: moving BuildEnvironment.vue after the links container, updating styles, and refactoring to accept BuildInfo prop.

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecovBot commented Feb 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@userquin
userquin marked this pull request as draft February 4, 2026 19:11

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
test/nuxt/components/AppFooter.spec.ts (1)

10-25: Prefer DOM assertions over full HTML string matches to reduce brittleness.

HTML string contains are fragile to class or attribute ordering changes. Consider asserting on elements/text/attributes instead.

♻️ Example adjustment
- const html = component.html()- expect(html).toContain('<span class="tracking-wider">dev</span>')- expect(html).toContain(- '<a href="https://github.com/npmx-dev/npmx.dev/commit/704987bba88909f3782d792c224bde989569acb9"',- )+ const env = component.find('span.tracking-wider')+ expect(env.exists()).toBe(true)+ expect(env.text()).toBe('dev')+ const commitLink = component.find('a[href*="commit/"]')+ expect(commitLink.exists()).toBe(true)
test/nuxt/components/BuildEnvironment.spec.ts (1)

7-22: Make the dev test self-contained by inlining BuildInfo.

Relying on useAppConfig() couples this test to Nuxt app initialisation. An explicit BuildInfo keeps it deterministic and focused.

♻️ Example adjustment
- const buildInfo = useAppConfig().buildInfo as BuildInfo+ const buildInfo: BuildInfo = {+ env: 'dev',+ version: '0.0.0',+ time: 1770237446424,+ commit: '704987bba88909f3782d792c224bde989569acb9',+ shortCommit: '704987b',+ branch: 'xxx',+ }

Comment threadmodules/build-env.ts
Comment threadconfig/env.ts Outdated
Comment threadtest/nuxt/components/AppFooter.spec.ts
Comment threadtest/nuxt/components/BuildEnvironment.spec.ts

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/nuxt/components/AppFooter.spec.ts (1)

6-16: Consider using DOM queries for more resilient assertions.

The exact HTML string matching (e.g., <span class="tracking-wider">) is fragile—if class order changes or attributes are added, the test will break unexpectedly. Using Vue Test Utils selectors would be more maintainable.

♻️ Suggested refactor using DOM queries
 it('BuildEnvironment is properly displayed at settings', async () => {
const buildInfo = useAppConfig().buildInfo
const component = await mountSuspended(AppFooter, {
route: '/settings',
})
- const html = component.html()- expect(html).toContain(`<span class="tracking-wider">${buildInfo.env}</span>`)- expect(html).toContain(- `<a href="https://github.com/npmx-dev/npmx.dev/commit/${buildInfo.commit}"`,- )+ const envSpan = component.find('span.tracking-wider')+ expect(envSpan.exists()).toBe(true)+ expect(envSpan.text()).toBe(buildInfo.env)+ const commitLink = component.find(`a[href*="${buildInfo.commit}"]`)+ expect(commitLink.exists()).toBe(true)
})

Comment threadtest/nuxt/components/BuildEnvironment.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Comment threadtest/nuxt/components/BuildEnvironment.spec.ts Outdated
@userquin

Copy link
Copy Markdown
MemberAuthor

Actionable comments posted: 1

🧹 Nitpick comments (1)

fixed with last commit

expect(envSpan.exists()).toBe(false)
const commitLink = component.find(`a[href$="/commit/${buildInfo.commit}"]`)
expect(commitLink.exists()).toBe(false)
const tagLink = component.find(`a[href$="/tag/v${buildInfo.commit}"]`)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Use version here

@userquin

Copy link
Copy Markdown
MemberAuthor

superseded by #1032

@userquinuserquin closed this Feb 5, 2026
@userquin
userquin deleted the fix-app-footer-layout branch February 5, 2026 19:24
Sign up for freeto 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

@userquin