CollectiveDetails modal Back link navigation #10551
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
It would be great to link to the original conversations/issues to better understand the problem we're trying to solve.
The history pattern is well implemented.
Without more context, it however looks like a complex approach to only partially solve the problem: as far as I understand it, what we want is the ability to quickly go back to the parent when clicking on a children from the collective details.
But then, why only look at the history? If I send you a link like https://opencollective-frontend-git-feat-navhistory-opencollective.vercel.app/dashboard/opensource/hosted-collectives/53kzxy4v-07wlr6m9-klepmj9n-o8agdbe5, you'll probably want to see a link at the top saying Go back to {parent}'s details.
A simpler approach would therefore be to simply replace the header with:
<div className='mb-10'>
<h1 className="text-2xl font-bold">
<FormattedMessage defaultMessage="Collective's overview" id="28uZ0u" />
</h1>
{isChild && (
<Link href={linkToParentModal} className="text-xs text-blue-700 hover:underline">
← Back to {collective.parent.name}
</Link>
)}
</div>| const history = | ||
| url === state.history.slice(-1)[0] | ||
| ? // Do not repeat the same URL | ||
| state.history | ||
| : url === state.history.slice(-2)[0] | ||
| ? // If returning, pop the last URL | ||
| state.history.slice(0, -1) | ||
| : // Else, keep the last 10 URLs visited | ||
| [...state.history, url].slice(-10); | ||
| return { history }; |
There was a problem hiding this comment.
🍨 Personal preference: I tend to find if/else if blocks cleaners than nested ternaries for this kind of long condition.
| @@ -462,6 +464,8 @@ const CollectiveDetails = ({ | |||
| const isHostedCollective = host && collective?.host?.id === host?.id; | |||
| const isLoading = loading || loadingCollectiveInfo; | |||
| const isChild = !!collective?.parent?.id; | |||
| const lastUrl = history[history.length - 2]; | |||
| const backToUrl = /\/dashboard\/[\w\d-]+\/(hosted|all)-collectives\/[\w\d-]{35}/i.test(lastUrl) ? lastUrl : null; | |||
There was a problem hiding this comment.
Testing string URLs with a regex is always a bit risky. A safer approach is to first parse it:
const parsedUrl = new URL(url); // TODO: Handle parse exceptions
const regex = /^{regexContent}$/; // Regex with start + end
return regex.test(parsedUrl.pathname);
|
@Betree this comes from different conversations and perhaps there's no silver bullet for this issue. In the past, we raised with @gustavlrsn the issue of drawer navigation and the requirement to have a generic way to move between multiple/different drawer contents. This is a generic issue: opening event details from a collective details drawer, opening collective details from an expense details drawer, etc... This implementation is a prototype to solve one of these issues, but I'm not sure if this is exactly what we want TBH. |
Do we have other cases today of drawers opening other drawers? |
|
@Betree not right now, but ideally:
|
Adds a "Back" button when navigating within the CollectiveDetails modal.
This is a naive prototype of navigation history.
simplescreenrecorder-2024-07-15_13.24.45.mp4