Skip to content
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

feat(breadcrumbs): Refactor breadcrumbs to remove path filter #186

Merged
merged 9 commits into from
Nov 7, 2023

Conversation

hannasage
Copy link

Purpose

Previously, breadcrumbs filtered out the options array with the following line:

.filter((option) => window.location.href.includes(option.to))

That means any crumb where the route wasn't present in the current path would be filtered out and not shown. This PR removes that filter and leaves the crumb config array in the hands of the developer since not every breadcrumb needs to be directly related to the path. This PR also refactors the config for the New Submission breadcrumb options so that it can be built dynamic via the current path.

Linked Issues to Close

None

Approach

A config's available items are explicitly the responsibility of the config. The component no longer will filter any out, it'll only sort them based on order.

Assorted Notes/Considerations/Learning

None

@hannasage hannasage requested a review from 13bfrancis November 6, 2023 19:13
@hannasage hannasage self-assigned this Nov 6, 2023
@hannasage hannasage marked this pull request as ready for review November 6, 2023 19:20
user?.isCms &&
user.user?.["custom:cms-roles"].includes(UserRoles.HELPDESK)
? "cmsStatus.keyword"
: "stateStatus.keyword",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops! Lemme remove this, this was leftover from helping Wale lol

return (
<SimplePageContainer>
<BreadCrumbs options={BREAD_CRUMB_CONFIG_NEW_SUBMISSION} />
<BreadCrumbs options={NEW_SUBMISSION_CRUMBS(location.pathname)} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this little function here to build out the option. I'm always a big fan of declarative code

Copy link
Collaborator

@13bfrancis 13bfrancis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it. Great work Kevin

@hannasage hannasage merged commit 35b4780 into master Nov 7, 2023
15 checks passed
@hannasage hannasage deleted the breadcrumb-refactor branch November 7, 2023 17:54
Copy link
Contributor

🎉 This PR is included in version 1.5.0-val.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants