Merged
Conversation
Contributor
|
@DanielCotoJ is attempting to deploy a commit to the ACTA Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an Impacta Bootcamp certificate UI (CertificateCanvas and Certificate), a Changes
Sequence DiagramsequenceDiagram
participant User as User
participant IssueUI as Issue UI\n(useIssueCredential)
participant ShareUI as Share UI\n(useShareCredential)
participant VerifyHook as Verify Hook\n(useCredentialVerify)
participant CertView as Certificate View\n(Certificate / CertificateCanvas)
User->>IssueUI: Issue Impacta credential
IssueUI->>IssueUI: Set vc.title = "Impacta Bootcamp Certificate"
User->>ShareUI: Request share
ShareUI->>ShareUI: Detect ImpactaCertificateCredential
ShareUI->>ShareUI: Build payload (issuer, subjectDid, type, issuedAt, status, holderName)
ShareUI->>ShareUI: POST /api/share (try) or Base64-encode fallback
User->>VerifyHook: Open share link
VerifyHook->>VerifyHook: Parse payload, set shareType
VerifyHook->>CertView: shareType indicates Impacta
CertView->>CertView: Render CertificateCanvas with holderName, year and details
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/components/modules/vault/ui/VaultDashboard.tsx (1)
267-298: Deduplicate active-credential construction.The same
setActivepayload is built twice (onClick/onView). Extract one helper to avoid drift and simplify maintenance.Refactor sketch
+ const openCredential = useCallback( + (credential: (typeof filteredCredentials)[number]) => { + const ac = actaById.get(credential.id); + if (!ac) return; + setActive({ + credential: ac, + gridCredential: { + id: credential.id, + name: credential.name, + category: credential.category, + status: credential.status, + wallet: getWalletFromDid(credential.username), + url: credential.url || undefined, + }, + }); + }, + [actaById, getWalletFromDid] + ); @@ - onClick={() => { - const ac = actaById.get(credential.id); - if (ac) { - setActive({ - credential: ac, - gridCredential: { - id: credential.id, - name: credential.name, - category: credential.category, - status: credential.status, - wallet: getWalletFromDid(credential.username), - url: credential.url || undefined, - }, - }); - } - }} + onClick={() => openCredential(credential)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/vault/ui/VaultDashboard.tsx` around lines 267 - 298, The onClick and onView handlers both build the same setActive payload (using actaById.get(credential.id), getWalletFromDid(credential.username) and the gridCredential fields), so extract a helper function (e.g., buildActiveForCredential or handleOpenCredential) that accepts the credential (or credential.id) and returns/calls setActive with the constructed object; then replace both onClick and onView to call that helper to eliminate duplication and keep logic centralized (ensure the helper references actaById, setActive and getWalletFromDid).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/modules/credentials/ui/SavedCredentialsCard.tsx`:
- Around line 39-43: The clickable motion.div in SavedCredentialsCard (layoutId
prop and onClick handler) is not keyboard-accessible—make it behave like a
button by adding role="button", tabIndex={0}, and an onKeyDown handler that
invokes the same onClick callback when Enter or Space is pressed (and
preventDefault for Space to avoid scrolling); also include an appropriate
aria-expanded or aria-pressed attribute if this toggles an expanded state so
assistive tech sees the state change. Ensure you only add these when onClick is
provided to preserve non-interactive semantics.
- Around line 103-113: SavedCredentialsCard currently calls new
URL(url).hostname directly which will throw for malformed or relative URLs; wrap
the hostname extraction in a try-catch inside the SavedCredentialsCard render
(or helper used by it) so parsing failures are caught and a safe fallback is
rendered (e.g., the original url string or an empty string). Specifically,
update the block that renders {new URL(url).hostname} to attempt new
URL(url).hostname inside try/catch (or use URL-safe parsing) and use the caught
error to fall back to a safe display value; reference the SavedCredentialsCard
component and the CredentialCardProps url prop when locating the change.
In `@src/components/modules/vault/ui/VaultDashboard.tsx`:
- Around line 81-82: The outside-click listener registered via
useOutsideClick(expandedRef, closeExpanded) is firing while the Raw JSON overlay
(showRawJson) is open and closing the parent view; update the outside-click
handler to ignore clicks when showRawJson is true (or when the click target is
inside the Raw JSON overlay/backdrop). Specifically, change the callback passed
to useOutsideClick (or closeExpanded itself) to first check the showRawJson
boolean and/or verify the event.target is not within the Raw JSON overlay
element before calling closeExpanded; alternatively only register
useOutsideClick when showRawJson is false so expandedRef.closeExpanded cannot be
triggered while the JSON modal is open.
- Around line 66-79: The effect in useEffect that sets
document.body.style.overflow based on active (and defines onKeyDown) doesn't
restore the body's overflow on cleanup, so if the component unmounts while
active scroll stays locked; update the effect (useEffect) to capture the
previous document.body.style.overflow value when running, set overflow =
'hidden' when active, and in the cleanup function both remove the window keydown
listener (onKeyDown) and restore document.body.style.overflow back to the saved
previous value (or 'auto' as fallback) to ensure scroll is always returned when
closeExpanded/unmount occurs.
---
Nitpick comments:
In `@src/components/modules/vault/ui/VaultDashboard.tsx`:
- Around line 267-298: The onClick and onView handlers both build the same
setActive payload (using actaById.get(credential.id),
getWalletFromDid(credential.username) and the gridCredential fields), so extract
a helper function (e.g., buildActiveForCredential or handleOpenCredential) that
accepts the credential (or credential.id) and returns/calls setActive with the
constructed object; then replace both onClick and onView to call that helper to
eliminate duplication and keep logic centralized (ensure the helper references
actaById, setActive and getWalletFromDid).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
next.config.tspackage.jsonsrc/components/modules/credentials/ui/CredentialCard.tsxsrc/components/modules/credentials/ui/SavedCredentialsCard.tsxsrc/components/modules/vault/ui/VaultDashboard.tsxsrc/hooks/use-outside-click.ts
💤 Files with no reviewable changes (1)
- src/components/modules/credentials/ui/CredentialCard.tsx
Comment on lines
+39
to
+43
| <motion.div | ||
| layoutId={layoutId ? `card-${layoutId}` : undefined} | ||
| onClick={onClick} | ||
| className={onClick ? 'cursor-pointer' : ''} | ||
| > |
There was a problem hiding this comment.
Clickable card is not keyboard-accessible.
At Line 41 this is pointer-only interaction on a div; keyboard users can’t open the expanded view. Add semantic/button-like keyboard handling.
Suggested fix
<motion.div
layoutId={layoutId ? `card-${layoutId}` : undefined}
onClick={onClick}
+ onKeyDown={(e) => {
+ if (!onClick) return;
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ onClick();
+ }
+ }}
+ role={onClick ? 'button' : undefined}
+ tabIndex={onClick ? 0 : undefined}
+ aria-label={onClick ? `Open credential ${name}` : undefined}
className={onClick ? 'cursor-pointer' : ''}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <motion.div | |
| layoutId={layoutId ? `card-${layoutId}` : undefined} | |
| onClick={onClick} | |
| className={onClick ? 'cursor-pointer' : ''} | |
| > | |
| <motion.div | |
| layoutId={layoutId ? `card-${layoutId}` : undefined} | |
| onClick={onClick} | |
| onKeyDown={(e) => { | |
| if (!onClick) return; | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| onClick(); | |
| } | |
| }} | |
| role={onClick ? 'button' : undefined} | |
| tabIndex={onClick ? 0 : undefined} | |
| aria-label={onClick ? `Open credential ${name}` : undefined} | |
| className={onClick ? 'cursor-pointer' : ''} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modules/credentials/ui/SavedCredentialsCard.tsx` around lines
39 - 43, The clickable motion.div in SavedCredentialsCard (layoutId prop and
onClick handler) is not keyboard-accessible—make it behave like a button by
adding role="button", tabIndex={0}, and an onKeyDown handler that invokes the
same onClick callback when Enter or Space is pressed (and preventDefault for
Space to avoid scrolling); also include an appropriate aria-expanded or
aria-pressed attribute if this toggles an expanded state so assistive tech sees
the state change. Ensure you only add these when onClick is provided to preserve
non-interactive semantics.
Comment on lines
+103
to
+113
| {url && ( | ||
| <a | ||
| href={url} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-xs text-blue-400 hover:text-blue-300 hover:underline block truncate" | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| {new URL(url).hostname} | ||
| </a> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and review the actual code
fd 'SavedCredentialsCard\.tsx$' src --exec cat -n {} | head -150Repository: ACTA-Team/dApp-ACTA
Length of output: 6914
🏁 Script executed:
# Search for useVaultCards and how credential URLs are handled
fd '(useVaultCards|VaultCards|credentials)' src --type=ts --type=tsx | head -20Repository: ACTA-Team/dApp-ACTA
Length of output: 232
🏁 Script executed:
# Check if there are any try-catch patterns around URL parsing in the codebase
rg -n 'new URL\(' src --type=ts --type=tsx -B2 -A2Repository: ACTA-Team/dApp-ACTA
Length of output: 90
🏁 Script executed:
# Find CredentialCardProps type definition
fd -e ts -e tsx -name "*.ts" -o -name "*.tsx" | xargs rg -l "CredentialCardProps" | head -5Repository: ACTA-Team/dApp-ACTA
Length of output: 399
🏁 Script executed:
# Search for the `@/`@types/credentials file
fd "credentials\.(ts|tsx)$" src/@typesRepository: ACTA-Team/dApp-ACTA
Length of output: 88
🏁 Script executed:
# Find where credentials are sourced from
rg -n "new URL\(" src --type-listRepository: ACTA-Team/dApp-ACTA
Length of output: 452
🏁 Script executed:
# Look for credential data sources and useVaultCards
fd -e ts -e tsx . src | xargs rg -l "useVaultCards|getCredentials|credential.*url" | head -10Repository: ACTA-Team/dApp-ACTA
Length of output: 165
🏁 Script executed:
# Check type definition file directly
find src/@types -name "*.ts" -o -name "*.tsx" | head -20Repository: ACTA-Team/dApp-ACTA
Length of output: 175
🏁 Script executed:
cat -n src/@types/credentials.tsRepository: ACTA-Team/dApp-ACTA
Length of output: 2504
🏁 Script executed:
cat -n src/components/modules/vault/hooks/useVaultCards.ts | head -100Repository: ACTA-Team/dApp-ACTA
Length of output: 2210
🏁 Script executed:
# Check if there are other usages of SavedCredentialsCard or CredentialCard
rg -n "SavedCredentialsCard|CredentialCard" src --type ts --type tsx 2>/dev/null | head -20Repository: ACTA-Team/dApp-ACTA
Length of output: 45
🏁 Script executed:
# Check if url is set anywhere else besides useVaultCards
rg -n "\.url\s*=" src --type ts --type tsx 2>/dev/null | grep -i credentialRepository: ACTA-Team/dApp-ACTA
Length of output: 45
🏁 Script executed:
# Search for any actual URL values in the vault-related code
rg -n "https?://" src/components/modules/vault --type ts --type tsx 2>/dev/nullRepository: ACTA-Team/dApp-ACTA
Length of output: 45
🏁 Script executed:
# Let's check how SavedCredentialsCard is actually used
rg -n "CredentialCard" src/components/modules/credentials --type ts --type tsx 2>/dev/nullRepository: ACTA-Team/dApp-ACTA
Length of output: 45
🏁 Script executed:
# Check the actual VaultDashboard to see how credentials are passed
cat -n src/components/modules/vault/ui/VaultDashboard.tsx | head -200Repository: ACTA-Team/dApp-ACTA
Length of output: 8645
🏁 Script executed:
# Continue reading VaultDashboard to see how credentials are rendered
cat -n src/components/modules/vault/ui/VaultDashboard.tsx | sed -n '200,400p'Repository: ACTA-Team/dApp-ACTA
Length of output: 10431
🏁 Script executed:
# Check if url is ever actually set in the credential data
rg -n "url:" src/components/modules/vault --type ts --type tsx 2>/dev/nullRepository: ACTA-Team/dApp-ACTA
Length of output: 45
🏁 Script executed:
# Look for where CredentialCard is instantiated with url prop
rg -B5 -A5 "CredentialCard" src/components/modules/vault/ui/VaultDashboard.tsxRepository: ACTA-Team/dApp-ACTA
Length of output: 1317
🏁 Script executed:
# Check the entire useVaultCards to see the full context
cat -n src/components/modules/vault/hooks/useVaultCards.tsRepository: ACTA-Team/dApp-ACTA
Length of output: 2210
🏁 Script executed:
# Verify if url is ever populated anywhere else in the credential flow
rg -n "url" src/components/modules/vault/hooks --type ts --type tsx 2>/dev/null | head -30Repository: ACTA-Team/dApp-ACTA
Length of output: 45
Add defensive parsing for URL hostname to handle future data changes.
Line 111 calls new URL(url).hostname without error handling. While url is currently always an empty string in the codebase (hardcoded in useVaultCards.ts line 31), the component's interface accepts any string via CredentialCardProps. If URL data is introduced in the future or the component is reused elsewhere, a malformed or relative URL would throw. Add a try-catch guard around the URL parsing.
Suggested fix
export function CredentialCard({
@@
}: ExpandableCredentialCardProps) {
+ const hostname = (() => {
+ if (!url) return null;
+ try {
+ return new URL(url).hostname;
+ } catch {
+ return null;
+ }
+ })();
@@
- {url && (
+ {url && hostname && (
<a
href={url}
target="_blank"
rel="noopener noreferrer"
className="text-xs text-blue-400 hover:text-blue-300 hover:underline block truncate"
onClick={(e) => e.stopPropagation()}
>
- {new URL(url).hostname}
+ {hostname}
</a>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modules/credentials/ui/SavedCredentialsCard.tsx` around lines
103 - 113, SavedCredentialsCard currently calls new URL(url).hostname directly
which will throw for malformed or relative URLs; wrap the hostname extraction in
a try-catch inside the SavedCredentialsCard render (or helper used by it) so
parsing failures are caught and a safe fallback is rendered (e.g., the original
url string or an empty string). Specifically, update the block that renders {new
URL(url).hostname} to attempt new URL(url).hostname inside try/catch (or use
URL-safe parsing) and use the caught error to fall back to a safe display value;
reference the SavedCredentialsCard component and the CredentialCardProps url
prop when locating the change.
Comment on lines
+66
to
+79
| useEffect(() => { | ||
| function onKeyDown(event: KeyboardEvent) { | ||
| if (event.key === 'Escape') closeExpanded(); | ||
| } | ||
|
|
||
| if (active) { | ||
| document.body.style.overflow = 'hidden'; | ||
| } else { | ||
| document.body.style.overflow = 'auto'; | ||
| } | ||
| }, [contentCred]); | ||
|
|
||
| window.addEventListener('keydown', onKeyDown); | ||
| return () => window.removeEventListener('keydown', onKeyDown); | ||
| }, [active, closeExpanded]); |
There was a problem hiding this comment.
Body scroll lock is not fully restored on unmount.
This effect sets document.body.style.overflow, but cleanup only removes the keydown listener. If unmounted while active, scroll can remain locked.
Suggested fix
useEffect(() => {
+ const previousOverflow = document.body.style.overflow;
+
function onKeyDown(event: KeyboardEvent) {
if (event.key === 'Escape') closeExpanded();
}
@@
window.addEventListener('keydown', onKeyDown);
- return () => window.removeEventListener('keydown', onKeyDown);
+ return () => {
+ window.removeEventListener('keydown', onKeyDown);
+ document.body.style.overflow = previousOverflow;
+ };
}, [active, closeExpanded]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modules/vault/ui/VaultDashboard.tsx` around lines 66 - 79, The
effect in useEffect that sets document.body.style.overflow based on active (and
defines onKeyDown) doesn't restore the body's overflow on cleanup, so if the
component unmounts while active scroll stays locked; update the effect
(useEffect) to capture the previous document.body.style.overflow value when
running, set overflow = 'hidden' when active, and in the cleanup function both
remove the window keydown listener (onKeyDown) and restore
document.body.style.overflow back to the saved previous value (or 'auto' as
fallback) to ensure scroll is always returned when closeExpanded/unmount occurs.
Comment on lines
+81
to
82
| useOutsideClick(expandedRef, closeExpanded); | ||
|
|
There was a problem hiding this comment.
Outside-click handler conflicts with the Raw JSON overlay.
When showRawJson is open, backdrop click at Line 504 should close only JSON, but the document outside-click listener can also call closeExpanded(), closing the parent view.
Suggested fix
+ const handleOutsideExpandedClick = useCallback(() => {
+ if (showRawJson) return;
+ closeExpanded();
+ }, [showRawJson, closeExpanded]);
+
- useOutsideClick(expandedRef, closeExpanded);
+ useOutsideClick(expandedRef, handleOutsideExpandedClick);Also applies to: 497-505
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modules/vault/ui/VaultDashboard.tsx` around lines 81 - 82, The
outside-click listener registered via useOutsideClick(expandedRef,
closeExpanded) is firing while the Raw JSON overlay (showRawJson) is open and
closing the parent view; update the outside-click handler to ignore clicks when
showRawJson is true (or when the click target is inside the Raw JSON
overlay/backdrop). Specifically, change the callback passed to useOutsideClick
(or closeExpanded itself) to first check the showRawJson boolean and/or verify
the event.target is not within the Raw JSON overlay element before calling
closeExpanded; alternatively only register useOutsideClick when showRawJson is
false so expandedRef.closeExpanded cannot be triggered while the JSON modal is
open.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx (1)
5-5: Use<section>instead of<main>for component reusability.The
<main>landmark element should typically appear once per page. While this component is currently unused, if it's intended as a reusable certificate display component, using<section>(or<div>) provides better flexibility and prevents landmark conflicts if mounted within a page that already declares a page-level<main>. Consider this refactor for future-proofing the component design.Suggested change
- <main className="min-h-screen bg-[`#0a0a0a`] flex items-center justify-center p-4"> + <section + aria-label="Certificado Impacta Bootcamp" + className="min-h-screen bg-[`#0a0a0a`] flex items-center justify-center p-4" + > ... - </main> + </section>Also applies to: 169-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx` at line 5, The Certificate component uses a <main> element for its wrapper which can create landmark conflicts when reused; update the component (Certificate in Certificate.tsx) to replace the outer <main className="min-h-screen bg-[`#0a0a0a`] flex items-center justify-center p-4"> with a <section> (or a div) element and also change the second occurrence noted at line 169 to the same element so the component becomes a non-page-level, reusable section.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`:
- Around line 63-105: The decorative Image elements in Certificate.tsx (e.g.,
the Image instances rendering asteriscs, corner-seal, circles, latam-outline)
expose verbose alt text; change each decorative Image to be ignored by assistive
tech by setting alt="" and adding aria-hidden="true" (or role="presentation") on
the Image component props, and apply the same change to the other decorative
Image usages referenced around lines 145-153 so screen readers skip them.
- Around line 137-141: The certificate year is hardcoded in the Certificate
component (the span rendering "2026"); change Certificate.tsx to accept a
year/date via props (e.g., add a prop like issueYear or issueDate in the
component's props/interface) and render that prop inside the span instead of the
literal "2026"; also update any places that instantiate Certificate to pass the
year, and consider a sensible fallback (e.g., current year) inside the component
if the prop is undefined.
---
Nitpick comments:
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`:
- Line 5: The Certificate component uses a <main> element for its wrapper which
can create landmark conflicts when reused; update the component (Certificate in
Certificate.tsx) to replace the outer <main className="min-h-screen bg-[`#0a0a0a`]
flex items-center justify-center p-4"> with a <section> (or a div) element and
also change the second occurrence noted at line 169 to the same element so the
component becomes a non-page-level, reusable section.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
public/assets/impacta-bootcamp/asteriscs.svgis excluded by!**/*.svgpublic/assets/impacta-bootcamp/circles.svgis excluded by!**/*.svgpublic/assets/impacta-bootcamp/corner-dots.svgis excluded by!**/*.svgpublic/assets/impacta-bootcamp/corner-seal.svgis excluded by!**/*.svgpublic/assets/impacta-bootcamp/impacta-logo.svgis excluded by!**/*.svgpublic/assets/impacta-bootcamp/latam-outline.svgis excluded by!**/*.svgpublic/assets/impacta-bootcamp/stellar-baf-trustless.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx
Comment on lines
+63
to
+105
| {/* Top: Row of Asteriscs */} | ||
| <div className="absolute top-[3.2%] left-[12.5%] w-[20%]"> | ||
| <Image | ||
| src="/assets/impacta-bootcamp/asteriscs.svg" | ||
| alt="Asteriscs decorativos" | ||
| width={244} | ||
| height={30} | ||
| className="w-full h-auto" | ||
| /> | ||
| </div> | ||
|
|
||
| {/* Top-right: Corner Seal (globe + asterisk) in white box */} | ||
| <div className="absolute top-[2%] right-[2%] w-[5.8%] bg-white p-[0.3%]"> | ||
| <Image | ||
| src="/assets/impacta-bootcamp/corner-seal.svg" | ||
| alt="Sello de esquina" | ||
| width={70} | ||
| height={120} | ||
| className="w-full h-auto" | ||
| /> | ||
| </div> | ||
|
|
||
| {/* Left side: Circles / Coil */} | ||
| <div className="absolute top-[24%] left-[3%] w-[18%]"> | ||
| <Image | ||
| src="/assets/impacta-bootcamp/circles.svg" | ||
| alt="Circulos decorativos" | ||
| width={232} | ||
| height={76} | ||
| className="w-full h-auto" | ||
| /> | ||
| </div> | ||
|
|
||
| {/* Right side: Latam Map Outline with dots */} | ||
| <div className="absolute top-[12%] right-[3%] w-[44%]"> | ||
| <Image | ||
| src="/assets/impacta-bootcamp/latam-outline.svg" | ||
| alt="Mapa de Latinoamerica" | ||
| width={505} | ||
| height={610} | ||
| className="w-full h-auto" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Hide decorative graphics from assistive technologies.
These assets are decorative (as noted by comments), but they currently expose verbose alt text. That creates unnecessary noise for screen-reader users.
Suggested change
- <Image
+ <Image
src="/assets/impacta-bootcamp/asteriscs.svg"
- alt="Asteriscs decorativos"
+ alt=""
+ aria-hidden="true"
width={244}
height={30}
className="w-full h-auto"
/>
...
- <Image
+ <Image
src="/assets/impacta-bootcamp/circles.svg"
- alt="Circulos decorativos"
+ alt=""
+ aria-hidden="true"
width={232}
height={76}
className="w-full h-auto"
/>
...
- <Image
+ <Image
src="/assets/impacta-bootcamp/latam-outline.svg"
- alt="Mapa de Latinoamerica"
+ alt=""
+ aria-hidden="true"
width={505}
height={610}
className="w-full h-auto"
/>
...
- <Image
+ <Image
src="/assets/impacta-bootcamp/corner-dots.svg"
- alt="Puntos decorativos"
+ alt=""
+ aria-hidden="true"
width={54}
height={53}
className="w-full h-auto"
/>Also applies to: 145-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`
around lines 63 - 105, The decorative Image elements in Certificate.tsx (e.g.,
the Image instances rendering asteriscs, corner-seal, circles, latam-outline)
expose verbose alt text; change each decorative Image to be ignored by assistive
tech by setting alt="" and adding aria-hidden="true" (or role="presentation") on
the Image component props, and apply the same change to the other decorative
Image usages referenced around lines 145-153 so screen readers skip them.
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx
Outdated
Show resolved
Hide resolved
…ial handling - Added a new page for the Impacta Bootcamp Certificate, integrating the Certificate component for display. - Enhanced the `useCredentialVerify` and `useShareCredential` hooks to support the new Impacta Certificate type, including additional fields for credential sharing. - Updated the `CredentialVerify` and `ShareCredentialModal` components to conditionally render based on the Impacta Certificate type. - Refactored the `Certificate` component to accept props for dynamic rendering of certificate details.
- Standardized code formatting in `useShareCredential`, `CredentialVerify`, `ShareCredentialModal`, and `Certificate` components for improved readability. - Removed unnecessary line breaks and adjusted indentation for consistency.
- Removed the fetch request for sharing credentials, simplifying the logic within the `useShareCredential` hook. - Adjusted payload handling to focus on encoding the credential data without making an API call.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/modules/credentials/hooks/useCredentialVerify.ts (1)
84-94:⚠️ Potential issue | 🟡 MinorReset
shareTypewhen share payload is absent.
shareTypeis only updated inside the object branch; if the share payload disappears, staleshareTypecan keep Impacta rendering active.Proposed fix
if (shareParam && typeof shareParam === 'object') { const sp = shareParam as { revealedFields?: Record<string, unknown>; statement?: unknown; proof?: string; ok?: boolean; type?: string; }; setRevealed(sp.revealedFields || null); setShareType(typeof sp.type === 'string' ? sp.type : null); const st = sp.statement; const hasSt = typeof st === 'object' && st && 'kind' in st && (st as { kind?: string }).kind !== 'none'; const hasProof = typeof sp.proof === 'string' && sp.proof.length > 0; const hasZk = Boolean(hasSt && hasProof); setHasZkProofInShare(hasZk); if (hasZk) { setZkStatement(st as ZkStatement); } else { setZkStatement(null); } // No auto-verification: status must be shown only after user clicks + } else { + setShareType(null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/hooks/useCredentialVerify.ts` around lines 84 - 94, The bug is that shareType (managed by setShareType) is only set when shareParam is an object, leaving a stale value when the share payload is removed; update the branch handling non-object or falsy shareParam in useCredentialVerify (where shareParam and setShareType are used) to explicitly clear/reset shareType (call setShareType(null)) and also clear revealed (setRevealed(null)) as appropriate so the component stops rendering as if a share is present.
♻️ Duplicate comments (1)
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx (1)
63-105:⚠️ Potential issue | 🟡 MinorHide decorative images from assistive technologies.
These images are decorative and should not be announced by screen readers; use empty
altplusaria-hidden="true"for those assets.Proposed fix pattern
<Image src="/assets/impacta-bootcamp/asteriscs.svg" - alt="Asteriscs decorativos" + alt="" + aria-hidden="true" width={244} height={30} className="w-full h-auto" /><Image src="/assets/impacta-bootcamp/circles.svg" - alt="Circulos decorativos" + alt="" + aria-hidden="true" width={232} height={76} className="w-full h-auto" /><Image src="/assets/impacta-bootcamp/corner-dots.svg" - alt="Puntos decorativos" + alt="" + aria-hidden="true" width={54} height={53} className="w-full h-auto" />Also applies to: 150-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx` around lines 63 - 105, The decorative Image elements inside the Certificate component (the Image instances rendering asteriscs.svg, corner-seal.svg, circles.svg, latam-outline.svg and the other decorative images at the later block around lines 150-158) should be hidden from assistive tech: change their alt text to an empty string ("") and add aria-hidden="true" to each Image element so screen readers skip them; update every decorative <Image> occurrence in Certificate.tsx to follow this pattern.
🧹 Nitpick comments (1)
src/components/modules/credentials/ui/ShareCredentialModal.tsx (1)
39-39: Harden Impacta type detection with an explicit type guard.This keeps modal logic robust across credential shape variations and avoids relying on implicit
.includesavailability.Proposed refactor
- const isImpactaCertificate = !!credential?.type.includes('ImpactaCertificateCredential'); + const typeValue = credential?.type as unknown; + const isImpactaCertificate = + typeof typeValue === 'string' + ? typeValue.includes('ImpactaCertificateCredential') + : Array.isArray(typeValue) + ? typeValue.includes('ImpactaCertificateCredential') + : false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/ui/ShareCredentialModal.tsx` at line 39, The current detection using isImpactaCertificate = !!credential?.type.includes('ImpactaCertificateCredential') is fragile; create an explicit type-guard function (e.g., isImpactaCertificateCredential) and use it in ShareCredentialModal instead of the inline includes call. The guard should accept the credential object and safely verify the shape (check credential is truthy, ensure credential.type is the expected type — string or array — before accessing includes, or check a known discriminant property) and narrow the type so isImpactaCertificate = isImpactaCertificateCredential(credential) is safe across credential shape variations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/modules/credentials/hooks/useShareCredential.ts`:
- Around line 19-20: The code checks Impacta with const isImpacta = typeof
c.type === 'string' && c.type.includes('ImpactaCertificateCredential') but later
serializes credential.type as-is, letting non-string types bypass Impacta
handling; fix by normalizing the type to a string everywhere: compute a single
normalizedType = String(c.type || '') (or use `${c.type ?? ''}`) and use
normalizedType for the isImpacta check and when building/serializing the share
payload (replace any direct uses of credential.type), and update the other
occurrence in this module (the second usage around the share/serialization
logic) to use the same normalizedType.
In `@src/components/modules/credentials/ui/CredentialVerify.tsx`:
- Around line 24-57: The Impacta branch currently returns early from
CredentialVerify, rendering ImpactaCertificate and skipping the shared "Verify
Proof" UI and its handlers; remove the early return and instead render
ImpactaCertificate inline within the existing CredentialVerify JSX flow so the
same proof verification controls and handlers (the Verify Proof button/component
and any functions like handleVerifyProof / verifyProof) are included for Impacta
shares as well, or explicitly invoke the same Verify Proof component used by the
non-Impacta path after rendering ImpactaCertificate.
---
Outside diff comments:
In `@src/components/modules/credentials/hooks/useCredentialVerify.ts`:
- Around line 84-94: The bug is that shareType (managed by setShareType) is only
set when shareParam is an object, leaving a stale value when the share payload
is removed; update the branch handling non-object or falsy shareParam in
useCredentialVerify (where shareParam and setShareType are used) to explicitly
clear/reset shareType (call setShareType(null)) and also clear revealed
(setRevealed(null)) as appropriate so the component stops rendering as if a
share is present.
---
Duplicate comments:
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`:
- Around line 63-105: The decorative Image elements inside the Certificate
component (the Image instances rendering asteriscs.svg, corner-seal.svg,
circles.svg, latam-outline.svg and the other decorative images at the later
block around lines 150-158) should be hidden from assistive tech: change their
alt text to an empty string ("") and add aria-hidden="true" to each Image
element so screen readers skip them; update every decorative <Image> occurrence
in Certificate.tsx to follow this pattern.
---
Nitpick comments:
In `@src/components/modules/credentials/ui/ShareCredentialModal.tsx`:
- Line 39: The current detection using isImpactaCertificate =
!!credential?.type.includes('ImpactaCertificateCredential') is fragile; create
an explicit type-guard function (e.g., isImpactaCertificateCredential) and use
it in ShareCredentialModal instead of the inline includes call. The guard should
accept the credential object and safely verify the shape (check credential is
truthy, ensure credential.type is the expected type — string or array — before
accessing includes, or check a known discriminant property) and narrow the type
so isImpactaCertificate = isImpactaCertificateCredential(credential) is safe
across credential shape variations.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/baf/page.tsxsrc/components/modules/credentials/hooks/useCredentialVerify.tssrc/components/modules/credentials/hooks/useShareCredential.tssrc/components/modules/credentials/ui/CredentialVerify.tsxsrc/components/modules/credentials/ui/ShareCredentialModal.tsxsrc/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsxsrc/components/modules/issue/hooks/useIssueCredential.ts
Comment on lines
+19
to
+20
| const isImpacta = typeof c.type === 'string' && c.type.includes('ImpactaCertificateCredential'); | ||
|
|
There was a problem hiding this comment.
Normalize credential.type to a string before Impacta detection and share serialization.
Current logic assumes string type for detection, but serializes credential.type as-is. Downstream verification only treats type as string, so non-string types can bypass the Impacta rendering path.
Proposed fix
- const isImpacta = typeof c.type === 'string' && c.type.includes('ImpactaCertificateCredential');
+ const normalizedType =
+ typeof c.type === 'string'
+ ? c.type
+ : Array.isArray(c.type)
+ ? c.type.join(' ')
+ : '';
+ const isImpacta = normalizedType.includes('ImpactaCertificateCredential');- if (credential?.type) payload.type = credential.type;
+ const rawType = credential?.type as unknown;
+ const normalizedType =
+ typeof rawType === 'string'
+ ? rawType
+ : Array.isArray(rawType)
+ ? rawType.join(' ')
+ : '';
+ if (normalizedType) payload.type = normalizedType;Also applies to: 116-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modules/credentials/hooks/useShareCredential.ts` around lines
19 - 20, The code checks Impacta with const isImpacta = typeof c.type ===
'string' && c.type.includes('ImpactaCertificateCredential') but later serializes
credential.type as-is, letting non-string types bypass Impacta handling; fix by
normalizing the type to a string everywhere: compute a single normalizedType =
String(c.type || '') (or use `${c.type ?? ''}`) and use normalizedType for the
isImpacta check and when building/serializing the share payload (replace any
direct uses of credential.type), and update the other occurrence in this module
(the second usage around the share/serialization logic) to use the same
normalizedType.
Comment on lines
+24
to
+57
| if (isImpacta) { | ||
| return ( | ||
| <ImpactaCertificate | ||
| holderName={ | ||
| typeof impactaRevealed.holderName === 'string' | ||
| ? (impactaRevealed.holderName as string) | ||
| : undefined | ||
| } | ||
| issuer={ | ||
| typeof impactaRevealed.issuer === 'string' | ||
| ? (impactaRevealed.issuer as string) | ||
| : undefined | ||
| } | ||
| subjectDid={ | ||
| typeof impactaRevealed.subject === 'string' | ||
| ? (impactaRevealed.subject as string) | ||
| : undefined | ||
| } | ||
| credentialType={ | ||
| typeof impactaRevealed.type === 'string' ? (impactaRevealed.type as string) : undefined | ||
| } | ||
| issuedAt={ | ||
| typeof impactaRevealed.issuedAt === 'string' | ||
| ? (impactaRevealed.issuedAt as string) | ||
| : undefined | ||
| } | ||
| status={ | ||
| typeof impactaRevealed.status === 'string' | ||
| ? (impactaRevealed.status as string) | ||
| : undefined | ||
| } | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Impacta branch removes ZK proof re-verification UI.
The early return bypasses the Verify Proof action entirely for Impacta shares, so proof-bearing links can no longer be revalidated in this path.
Proposed fix
if (isImpacta) {
return (
- <ImpactaCertificate
- holderName={
- typeof impactaRevealed.holderName === 'string'
- ? (impactaRevealed.holderName as string)
- : undefined
- }
- issuer={
- typeof impactaRevealed.issuer === 'string'
- ? (impactaRevealed.issuer as string)
- : undefined
- }
- subjectDid={
- typeof impactaRevealed.subject === 'string'
- ? (impactaRevealed.subject as string)
- : undefined
- }
- credentialType={
- typeof impactaRevealed.type === 'string' ? (impactaRevealed.type as string) : undefined
- }
- issuedAt={
- typeof impactaRevealed.issuedAt === 'string'
- ? (impactaRevealed.issuedAt as string)
- : undefined
- }
- status={
- typeof impactaRevealed.status === 'string'
- ? (impactaRevealed.status as string)
- : undefined
- }
- />
+ <div className="w-full flex flex-col items-center justify-center py-6 gap-6">
+ <ImpactaCertificate
+ holderName={
+ typeof impactaRevealed.holderName === 'string'
+ ? (impactaRevealed.holderName as string)
+ : undefined
+ }
+ issuer={
+ typeof impactaRevealed.issuer === 'string'
+ ? (impactaRevealed.issuer as string)
+ : undefined
+ }
+ subjectDid={
+ typeof impactaRevealed.subject === 'string'
+ ? (impactaRevealed.subject as string)
+ : undefined
+ }
+ credentialType={
+ typeof impactaRevealed.type === 'string'
+ ? (impactaRevealed.type as string)
+ : undefined
+ }
+ issuedAt={
+ typeof impactaRevealed.issuedAt === 'string'
+ ? (impactaRevealed.issuedAt as string)
+ : undefined
+ }
+ status={
+ typeof impactaRevealed.status === 'string'
+ ? (impactaRevealed.status as string)
+ : undefined
+ }
+ />
+ {hasZkProofInShare && (
+ <button
+ onClick={reverify}
+ disabled={reverifyLoading}
+ className="rounded-lg border border-[`#edeed1`]/30 bg-transparent hover:bg-[`#edeed1`]/10 text-white px-4 py-2 text-sm font-medium disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
+ >
+ {reverifyLoading ? 'Verifying…' : 'Verify Proof'}
+ </button>
+ )}
+ </div>
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modules/credentials/ui/CredentialVerify.tsx` around lines 24 -
57, The Impacta branch currently returns early from CredentialVerify, rendering
ImpactaCertificate and skipping the shared "Verify Proof" UI and its handlers;
remove the early return and instead render ImpactaCertificate inline within the
existing CredentialVerify JSX flow so the same proof verification controls and
handlers (the Verify Proof button/component and any functions like
handleVerifyProof / verifyProof) are included for Impacta shares as well, or
explicitly invoke the same Verify Proof component used by the non-Impacta path
after rendering ImpactaCertificate.
…nd sharing functionality - Introduced floating animations for decorative elements in the CertificateCanvas component using CSS keyframes. - Added state management for sharing the certificate URL via social media, utilizing the useEffect hook to capture the current window URL. - Updated layout and styling for improved responsiveness and visual appeal.
- Reformatted the share link logic in the Certificate component for better clarity and maintainability. - Ensured consistent indentation and line breaks for improved code structure.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx (2)
22-22:⚠️ Potential issue | 🟡 MinorAvoid hardcoded year fallback.
Using
2026as fallback will go stale. Use a dynamic current-year fallback.Suggested fix
-export function CertificateCanvas({ holderName, year = 2026 }: CertificateCanvasProps) { +export function CertificateCanvas({ + holderName, + year = new Date().getFullYear(), +}: CertificateCanvasProps) { @@ const derivedYear = typeof year !== 'undefined' ? year : issuedAt && !Number.isNaN(Date.parse(issuedAt)) ? new Date(issuedAt).getFullYear() - : 2026; + : new Date().getFullYear();Also applies to: 211-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx` at line 22, The CertificateCanvas component (and the other occurrence around lines 211-216) currently uses a hardcoded fallback year 2026; replace that static default with a dynamic current-year fallback by computing new Date().getFullYear() at runtime and using it as the default for the year parameter (e.g., change the default for CertificateCanvas's year prop and the other certificate-rendering function/prop to use the computed current year) so the certificate never becomes stale.
67-73:⚠️ Potential issue | 🟠 MajorHide decorative images from assistive tech.
These assets are decorative; keep them out of the accessibility tree (
alt=""+aria-hidden="true").Suggested fix
<div className="absolute top-[3.2%] left-[12.5%] w-[20%] float-soft-delayed"> <Image src="/assets/impacta-bootcamp/asteriscs.svg" - alt="Asteriscs decorativos" + alt="" + aria-hidden="true" width={244} height={30} className="w-full h-auto" /> </div> @@ <div className="absolute top-[2%] right-[2%] w-[5.8%] bg-white p-[0.3%] float-soft"> <Image src="/assets/impacta-bootcamp/corner-seal.svg" - alt="Sello de esquina" + alt="" + aria-hidden="true" width={70} height={120} className="w-full h-auto" /> </div> @@ <div className="absolute top-[24%] left-[3%] w-[18%] float-soft-delayed"> <Image src="/assets/impacta-bootcamp/circles.svg" - alt="Circulos decorativos" + alt="" + aria-hidden="true" width={232} height={76} className="w-full h-auto" /> </div> @@ <div className="absolute top-[12%] right-[3%] w-[44%] float-soft"> <Image src="/assets/impacta-bootcamp/latam-outline.svg" - alt="Mapa de Latinoamerica" + alt="" + aria-hidden="true" width={505} height={610} className="w-full h-auto" /> </div> @@ <div className="absolute bottom-[14%] right-[1.5%] w-[4.5%] float-soft-delayed"> <Image src="/assets/impacta-bootcamp/corner-dots.svg" - alt="Puntos decorativos" + alt="" + aria-hidden="true" width={54} height={53} className="w-full h-auto" /> </div>Also applies to: 78-84, 89-95, 100-106, 154-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx` around lines 67 - 73, Update the decorative Image elements in the Certificate component so they are hidden from assistive technology: for each decorative <Image /> instance (the ones using "/assets/impacta-bootcamp/asteriscs.svg" and similar at the noted blocks) set alt="" and add aria-hidden="true" (and remove any meaningful alt text) so they are excluded from the accessibility tree; make this change for the Image usages around lines mentioned (the repeated decorative images in Certificate.tsx).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`:
- Line 3: The component currently uses useState and a useEffect to set shareUrl
(created from window.location and certificate data) which is unnecessary; remove
the shareUrl state and its useEffect and instead compute shareUrl synchronously
inside the Certificate component (e.g., a const shareUrl =
`${window.location.origin}/...${certificate.id}` or using the same logic
previously in the effect). Update any references that used shareUrl to use this
derived const and remove the imported useState/useEffect if no longer used.
---
Duplicate comments:
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`:
- Line 22: The CertificateCanvas component (and the other occurrence around
lines 211-216) currently uses a hardcoded fallback year 2026; replace that
static default with a dynamic current-year fallback by computing new
Date().getFullYear() at runtime and using it as the default for the year
parameter (e.g., change the default for CertificateCanvas's year prop and the
other certificate-rendering function/prop to use the computed current year) so
the certificate never becomes stale.
- Around line 67-73: Update the decorative Image elements in the Certificate
component so they are hidden from assistive technology: for each decorative
<Image /> instance (the ones using "/assets/impacta-bootcamp/asteriscs.svg" and
similar at the noted blocks) set alt="" and add aria-hidden="true" (and remove
any meaningful alt text) so they are excluded from the accessibility tree; make
this change for the Image usages around lines mentioned (the repeated decorative
images in Certificate.tsx).
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx
Outdated
Show resolved
Hide resolved
…hook and update Certificate component - Implemented a fallback mechanism in the `useShareCredential` hook to handle sharing via an API, with inline encoding as a backup. - Updated the share link format in the Certificate component to include a more descriptive message for social media sharing, improving user engagement.
…mp Certificate - Updated the share link message in the Certificate component to ensure proper encoding and formatting for social media sharing. - Maintained consistency with previous enhancements to the sharing functionality.
- Replaced useEffect with a more concise useState initialization for the share URL, improving code clarity and performance. - Ensured the share URL is set correctly based on the window location, maintaining functionality for social media sharing.
- Simplified the initialization of the share URL state in the Certificate component for improved readability. - Ensured the share URL is correctly set based on the window location, maintaining existing functionality.
… Certificate component - Updated the share URL management in the Certificate component to utilize useSyncExternalStore, enhancing performance and clarity. - Maintained the functionality of dynamically setting the share URL based on the window location.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx (1)
65-74:⚠️ Potential issue | 🟡 MinorHide decorative graphics from assistive technologies.
Decorative images (asterisks, circles, map outline, corner dots) expose verbose alt text that creates noise for screen-reader users. Set
alt=""andaria-hidden="true"on purely decorativeImagecomponents.Suggested fix for decorative images
<Image src="/assets/impacta-bootcamp/asteriscs.svg" - alt="Asteriscs decorativos" + alt="" + aria-hidden="true" width={244} height={30} className="w-full h-auto" />Apply the same pattern to:
- Line 91:
circles.svg- Line 102:
latam-outline.svg- Line 156:
corner-dots.svg- Line 80:
corner-seal.svg(if purely decorative)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx` around lines 65 - 74, In the Certificate React component, several decorative Image elements (e.g., the asteriscs, circles, latam-outline, corner-dots, and potentially corner-seal) expose verbose alt text; update those Image usages in Certificate.tsx to be ignored by assistive tech by setting alt="" and adding aria-hidden="true" on each decorative Image (identify the images by their src values: "/assets/impacta-bootcamp/asteriscs.svg", "circles.svg", "latam-outline.svg", "corner-dots.svg", and "corner-seal.svg" where applicable) so screen readers won’t read them.
🧹 Nitpick comments (1)
src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx (1)
258-286: Consider disabling share buttons when URL is unavailable.When
shareUrlis empty (during SSR), the buttons render withhref="#"which can confuse users. Consider adding visual disabled state or hiding the buttons until the URL is available.Optional improvement
<a href={ shareUrl ? `https://twitter.com/intent/tweet?text=${encodeURIComponent( `Mi certificado Impacta Bootcamp\n\n${shareUrl}\n\nEmitido por `@ActaXyz` \nGracias a `@TheBAFNetwork` `@TrustlessWork` `@StellarOrg` por el apoyo` )}` : '#' } target="_blank" rel="noopener noreferrer" - className="flex-1 flex items-center justify-center gap-2 py-2.5 px-3 rounded-lg bg-white/10 hover:bg-white/15 border border-white/20 text-white text-sm font-medium transition-colors" + className={`flex-1 flex items-center justify-center gap-2 py-2.5 px-3 rounded-lg border border-white/20 text-white text-sm font-medium transition-colors ${ + shareUrl ? 'bg-white/10 hover:bg-white/15' : 'bg-white/5 opacity-50 pointer-events-none' + }`} + aria-disabled={!shareUrl} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx` around lines 258 - 286, Certificate component currently renders Twitter and Linkedin <a> buttons with href="#" when shareUrl is falsy; update the two anchor elements (the Twitter button containing <Twitter/> and span "Share X", and the Linkedin button containing <Linkedin/> and span "Share LinkedIn") to either not render until shareUrl is available or render a visually disabled, non-clickable state: remove or avoid setting href when shareUrl is falsy, add aria-disabled="true" and a disabled CSS class (e.g., reduced opacity and pointer-events-none) and a title explaining the button is unavailable, and ensure any onClick handlers early-return if shareUrl is missing so no navigation occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`:
- Around line 65-74: In the Certificate React component, several decorative
Image elements (e.g., the asteriscs, circles, latam-outline, corner-dots, and
potentially corner-seal) expose verbose alt text; update those Image usages in
Certificate.tsx to be ignored by assistive tech by setting alt="" and adding
aria-hidden="true" on each decorative Image (identify the images by their src
values: "/assets/impacta-bootcamp/asteriscs.svg", "circles.svg",
"latam-outline.svg", "corner-dots.svg", and "corner-seal.svg" where applicable)
so screen readers won’t read them.
---
Nitpick comments:
In `@src/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx`:
- Around line 258-286: Certificate component currently renders Twitter and
Linkedin <a> buttons with href="#" when shareUrl is falsy; update the two anchor
elements (the Twitter button containing <Twitter/> and span "Share X", and the
Linkedin button containing <Linkedin/> and span "Share LinkedIn") to either not
render until shareUrl is available or render a visually disabled, non-clickable
state: remove or avoid setting href when shareUrl is falsy, add
aria-disabled="true" and a disabled CSS class (e.g., reduced opacity and
pointer-events-none) and a title explaining the button is unavailable, and
ensure any onClick handlers early-return if shareUrl is missing so no navigation
occurs.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/modules/credentials/hooks/useShareCredential.tssrc/components/modules/credentials/ui/impacta-bootcamp/Certificate.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/modules/credentials/hooks/useShareCredential.ts
…rtificate sharing - Implemented a new method to generate compact, URL-safe identifiers for share links in the Certificate component. - Added functionality to download the certificate as an image, including QR code generation for easy sharing. - Updated the CertificateCanvas component to support ref forwarding for better integration with the download feature. - Removed the unused BafPage component to streamline the codebase.
…nents - Removed an unused line from the ImpactaCertificateCanvasProps type definition. - Streamlined the Certificate component by eliminating redundant comments and ensuring consistent formatting for improved readability.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 ACTA Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
Credential Card & UI Enhancements:
CredentialCardinSavedCredentialsCard.tsxto usemotionfor animated expansionlayoutIdandonClickfor improved interactivityCredentialCardcomponent fromCredentialCard.tsxVault Dashboard:
VaultDashboard.tsxwith animated credential card expansion usingmotionandAnimatePresenceactivestate for expanded card viewConfiguration & Dependencies:
baseline-browser-mappingas a dev dependencynext.config.ts: removedtranspilePackagesand setoutputFileTracingRoottoprocess.cwd()Thank you for contributing to ACTA! We hope you can continue contributing to this project.
Summary by CodeRabbit