Uh oh!
There was an error while loading. Please reload this page.
fix: include link for eID App - #653
Conversation
📝 WalkthroughWalkthroughConverted the non-mobile QR login prompt's “eID App” text into an external hyperlink to the app store (opens in a new tab with proper rel/aria attributes) and changed a CTA button's gradient utility class name in the same auth page file. Changes
Sequence Diagram(s)(omitted — change is a localized UI tweak without multi-component control-flow requiring a sequence diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @platforms/pictique/src/routes/(auth)/auth/+page.svelte:
- Around line 154-156: The anchor that calls getAppStoreLink() uses
target="_blank" without rel attributes; update the <a> element wrapping
getAppStoreLink() to include rel="noopener noreferrer" to prevent window.opener
exposure and add an aria-label (eg. aria-label="Open eID App store in new tab")
to indicate it opens in a new tab for accessibility; ensure the change is
applied to the anchor containing getAppStoreLink() and preserve the existing
target="_blank" and styling.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
4-6: Remove unused constant imports.The constants
PUBLIC_APP_STORE_EID_WALLETandPUBLIC_PLAY_STORE_EID_WALLETare imported but never used. ThegetAppStoreLink()function (lines 24-36) uses hardcoded app store URLs instead of these environment variables.
🧹 Nitpick comments (1)
platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
152-152: Consider addingrel="noopener noreferrer"to the mobile branch for consistency.While the mobile branch (line 152) doesn't use
target="_blank", if you later decide to open links in new tabs on mobile as well, remember to include therelattribute for security. The current mobile implementation is fine since app store links typically launch the native app store.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platforms/pictique/src/routes/(auth)/auth/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @platforms/pictique/src/routes/(auth)/auth/+page.svelte:
- Line 173: Replace the invalid Tailwind class "bg-linear-to-r" with the correct
"bg-gradient-to-r" in the element that currently has class="rounded-xl
bg-linear-to-r from-[#4D44EF] via-[#F35B5B] to-[#F7A428] px-8 py-4 text-base
font-semibold text-white transition-opacity hover:opacity-90" so the gradient
utilities (from/via/to) render properly; search for the "bg-linear-to-r" token
in the +page.svelte component and update it to "bg-gradient-to-r".
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platforms/pictique/src/routes/(auth)/auth/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
154-159: LGTM! Successfully addresses issue #639.The "eID App" text is now properly wrapped in a hyperlink with all necessary security and accessibility attributes. This resolves the issue where the text was not clickable on desktop browsers.
Uh oh!
There was an error while loading. Please reload this page.
Description of change
Use the proper getAppStoreLink() as in the other platforms to get the right link for the eID app.
Issue Number
Closes#639
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.