feat: put env string next to 'npmx'#657
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
I like it. I wouldn't put anything for prod builds. and I wonder if it would be better in the header somewhere? next to the logo? otherwise this env flag only shows on the landing page and not on the search, compare, setting, etc. pages. |
|
this is a nice idea. I think it needs a design tweak as italic monospace isn't the most attractive font - I'll take a look |
2d7b810 to
77c3971
Compare
📝 WalkthroughWalkthroughThe PR reads Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/pages/index.vue (2)
25-25: No guard against missingenvvalue.If
buildInfoorenvisundefined(e.g. during local dev or a misconfigured build), destructuring will silently yieldundefined, and the<sup>on line 57 will render an empty but still-visible element. Consider a fallback or av-ifguard on the<sup>.Also, this ternary (
env === 'release' ? 'alpha' : env) is duplicated verbatim inAppHeader.vue. Extract a small computed or utility (e.g.useEnvLabel()) to keep the mapping in one place.Proposed: extract shared helper and guard rendering
For example, create a composable like
composables/useEnvLabel.ts:export function useEnvLabel() { const { env } = useAppConfig().buildInfo const envLabel = computed(() => { if (!env || env === 'production') return undefined return env === 'release' ? 'alpha' : env }) return { envLabel } }Then in both templates, use it with a
v-if:- <sup class="text-3xl italic text-fg-muted">{{ env === 'release' ? 'alpha' : env }}</sup> + <sup v-if="envLabel" class="text-3xl italic text-fg-muted">{{ envLabel }}</sup>
57-57: Thetext-3xlsize feels oversized for a superscript badge.On the index page this renders at
text-3xlwhereasAppHeader.vueusestext-sm. The visual weight difference is intentional (hero vs header), buttext-3xlfor a<sup>is quite large — danielroe's review comment also flagged the styling. Worth a second look at sizing/font choice here.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Yeah this is a great idea! Definitely solves a real issue! ❤️ |
|
Maybe we could show only 'dev' and 'preview' for now until we come up with better design so that we can reduce development friction? |
|
I removed 'alpha' and created the issue for better desgin #1135. |
7a1d5a8 to
e78169d
Compare
|
@shuuji3 Thanks for the implementation and the idea ❤️ I adjusted the styles a bit to improve a bit general view on different devices, if you have any other UI ideas, feel free to ping me, I'll be happy to discuss and help |
|
@alexdln Thanks, it looks good to me! I like colorizing it with accent colors 🎨 |

How about adding the env name ('dev', 'preview', 'canary', or 'alphe' (for 'release' env)) next to the "npmx" like Elk (https://main.elk.zone vs https://elk.zone) in addition to the footer?
The logo color is already different but sometimes not enough to distinguish (ref. https://discord.com/channels/1464542801676206113/1464544843740217455/1467534525218295889 😄)
We could drop "alpha" for production if it's preferable.
Closes #1135