-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENG-3859 feat(portal): better navigation between list details list identity and tag claim #822
ENG-3859 feat(portal): better navigation between list details list identity and tag claim #822
Conversation
ENG-3859 Navigation between List Details, List Identity and Tag Claim
Description: This is in reference to the ticket created by MK. PRO-27 This ticket is only to solve the navigation between the pages. Attachments: Problem Screenshots Screenshot 2024-09-03 at 2.34.01 PM.png Screenshot 2024-09-03 at 2.34.13 PM.png Screenshot 2024-09-03 at 2.34.50 PM.png Solution: Meta Data in Tag Claims and List Identity have a link to the List Details page. Navigation between List Details, List Identity and Tag Claim - Features (Figma) // Test Lead Notes // |
…ls-list-identity-and-tag-claim
@@ -79,6 +85,21 @@ export async function loader({ request, params }: LoaderFunctionArgs) { | |||
throw new Response('Not Found', { status: 404 }) | |||
} | |||
|
|||
let list: ClaimPresenter | null = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we want to start moving away from using let
we could try something like this --
const list = await ClaimsService.searchClaims({
predicate: getSpecialPredicate(CURRENT_ENV).tagPredicate.id,
object: identity.id,
})
.then(listResult => listResult?.data[0] ?? null)
.catch(error => {
logger('Failed to fetch list:', error);
return null;
});
what do you think? i know we're mixing these patterns in our loaders so likely a bigger question but wondering what you think.
not blocking just a question
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way works for me, I don't really have a preference. We should just have a quick chat and land on one or the other.
Looks good -- small question for us to consider |
Affected Packages
Apps
Packages
Tools
Overview
This adds navigation elements to detail pages which link to any associated lists. This might change a bit after we make some modifications to lists, but in the meantime, this'll do.
Screen Captures
If applicable, add screenshots or screen captures of your changes.
Declaration
PR Summary by Typo
Summary
This pull request introduces a new
DetailInfoCard
component and updates imports in various files. It replacesInfoCard
withDetailInfoCard
in some components and adds new properties and imports.Key Points
DetailInfoCard
component added inapps/portal/app/components/detail-info-card.tsx
InfoCard
withDetailInfoCard
inClaimDetails
and imports new dependenciesIdentityLoaderData
interface anduseLiveLoader
hook to loadDetailInfoCard
and newlist
property.To turn off PR summary, please visit Notification settings.