-
Notifications
You must be signed in to change notification settings - Fork 17
Replaced breadcrumb id's with their relevant name for better UX #1109
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
Conversation
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
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.
Caution
Changes requested ❌
Reviewed everything up to f7704df in 2 minutes and 33 seconds. Click for details.
- Reviewed
251lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/components/breadcrumbs.tsx:51
- Draft comment:
Ensure items[1] exists before accessing its title to avoid errors when the array has fewer than two elements. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The code is structured to only show this branch when there's a specific URL pattern for organizations. Given the URL pattern check and that breadcrumbs typically mirror the URL structure, it's reasonable to assume items[1] will exist in this case. Without seeing the useBreadcrumbs implementation, we can't be 100% certain, but the code structure strongly suggests this is intentional. I don't have visibility into the useBreadcrumbs hook implementation - what if it doesn't always match URL structure? What if there's an edge case where it returns fewer items? While those are valid concerns, the explicit URL pattern check (isOrganizationUUIDPath) suggests this is a well-defined case where the structure is known. If there was a mismatch, it would likely be caught in testing. The comment should be deleted as it's making assumptions without strong evidence of an actual issue, and the code structure suggests this access is intentional.
2. nextjs/src/features/organization/components/OrganizationDashboard.tsx:78
- Draft comment:
Review the use of router.push inside startTransition; ensure this delayed navigation aligns with the intended user experience. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that the use of router.push inside startTransition aligns with the intended user experience. This is a request for confirmation of intention, which is not allowed according to the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.
3. nextjs/src/features/organization/components/OrganizationDashboard.tsx:44
- Draft comment:
Typo: 'selecteDropdownOrgId' may be intended as 'selectedDropdownOrgId' or 'selectDropdownOrgId'. Please review the variable name. - Reason this comment was not posted:
Comment was on unchanged code.
4. nextjs/src/features/organization/components/OrganizationDashboard.tsx:79
- Draft comment:
Typo: The variable nameuseOrgIdin the template literal might be incorrect. Given the context (usingorgIdelsewhere), this might be a typo and should possibly beorgId. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_r93KRi0XaneKOGEV
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
nextjs/src/features/organization/components/OrganizationDashboard.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
|
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.
Important
Looks good to me! 👍
Reviewed 8774243 in 52 seconds. Click for details.
- Reviewed
32lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/features/organization/components/OrganizationDashboard.tsx:60
- Draft comment:
Remove the commented-out 'setLoading(true)' call if it’s no longer needed to keep the code clean. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. nextjs/src/features/organization/components/OrganizationDashboard.tsx:81
- Draft comment:
Remove the commented-out 'setLoading(false)' call after the error check to avoid dead code. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. nextjs/src/features/organization/components/OrganizationDashboard.tsx:96
- Draft comment:
Remove the commented-out 'setLoading(false)' call in fetchOrganizationDashboardDetails if it’s not intended for future use. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_aPHMToRyUGgZ2vpN
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.



What
Changed the bread crumb id's to its relevant name


changed it as discussed in meeting to replace organization with overview
Important
Replaced breadcrumb IDs with names, added copy functionality, and improved navigation in
OrganizationDashboardandSchemaCard.breadcrumbs.tsx.UUID_REGEXto identify organization UUID paths.useTransitionfor smoother navigation inOrganizationDashboard.tsx.useEffectanduseTransition.aliasquery parameter to schema URL inSchemaCard.tsx.UUID_REGEXtoCommonConstant.ts.This description was created by
for 8774243. You can customize this summary. It will automatically update as commits are pushed.