-
Notifications
You must be signed in to change notification settings - Fork 15
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: add back button to action sidebar #4016
Conversation
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.
@CzarekDryl Nice work, it works great.
The only thing not working as expected is the browser back button requires two clicks before the sidepanel opens up.
All back arrow buttons working as of spec:
@arrenv Browser back button is working fine now Screen.Recording.2025-01-03.at.11.56.28.mov |
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.
Hey @CzarekDryl nice start on this issue 💯
The back navigation on the action sidebar seems to be working as expected
Screen.Recording.2025-01-07.at.13.51.55.mov
However, I did leave some refactoring comments if you could be so kind to tackle them 🥇
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.
Is there any benefit of having this function as a hook? I'd suggest moving it to the utils.ts
Also, I would suggest refactoring this function to make use of the existing
GROUP_ADMIN_LIST, GROUP_FUNDS_LIST, GROUP_TEAMS_LIST
for the Action.ManageColony
and GROUP_LIST
for the Action.PaymentGroup
, as we might want to add more actions to each group in the future and this will make it easier for keeping groups and actions in sync.
Thus, I'd suggest the following
const getAvailableActions = (list: GroupListItem[]) =>
list.filter((item) => !item.isHidden).map((item) => item.action);
const ManageColonyActions = getAvailableActions([
...GROUP_ADMIN_LIST,
...GROUP_FUNDS_LIST,
...GROUP_TEAMS_LIST,
]);
const PaymentActions = getAvailableActions(GROUP_LIST);
export const getActionGroup = (actionType: Action) => {
if (!actionType) {
return null;
}
if (ManageColonyActions.includes(actionType)) return Action.ManageColony;
if (PaymentActions.includes(actionType)) {
return Action.PaymentGroup;
}
return null;
};
{actionGroupType && ( | ||
<button | ||
type="button" | ||
className="flex items-center justify-center py-2.5 text-gray-400 transition sm:hover:text-blue-400" | ||
onClick={() => { | ||
if (transactionId) { | ||
closeSidebarClick(); | ||
|
||
setTimeout(() => { | ||
toggleActionSidebarOn({ | ||
[ACTION_TYPE_FIELD_NAME]: actionGroupType, | ||
}); | ||
}, 500); | ||
|
||
return; | ||
} | ||
|
||
toggleActionSidebarOn({ | ||
[ACTION_TYPE_FIELD_NAME]: actionGroupType, | ||
}); | ||
}} | ||
> | ||
<ArrowLeft size={18} /> | ||
</button> | ||
)} |
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.
Seems the ActionSidebar
is increasing more and more, so let's clean this up a bit.
Let's create a partial component - for eg. GoBackButton
import { ArrowLeft } from '@phosphor-icons/react';
import React, { type FC } from 'react';
import { useActionSidebarContext } from '~context/ActionSidebarContext/ActionSidebarContext.ts';
import { type ColonyAction } from '~types/graphql.ts';
import { ACTION_TYPE_FIELD_NAME } from '../consts.ts';
import { getActionGroup, mapActionTypeToAction } from '../utils.ts';
interface GoBackButtonProps {
action?: ColonyAction | null;
onClick?: () => void;
}
export const GoBackButton: FC<GoBackButtonProps> = ({ action, onClick }) => {
const { actionSidebarToggle, actionSidebarInitialValues } =
useActionSidebarContext();
const { toggleOn: toggleActionSidebarOn } = actionSidebarToggle[1];
const actionType = mapActionTypeToAction(action);
const actionGroupType = getActionGroup(
actionSidebarInitialValues?.[ACTION_TYPE_FIELD_NAME] || actionType,
);
const openGroupedAction = () => {
toggleActionSidebarOn({
[ACTION_TYPE_FIELD_NAME]: actionGroupType,
});
};
if (!actionGroupType) {
return null;
}
return (
<button
type="button"
className="flex items-center justify-center py-2.5 text-gray-400 transition sm:hover:text-blue-400"
onClick={() => {
if (onClick) {
onClick();
setTimeout(openGroupedAction, 500);
return;
}
openGroupedAction();
}}
>
<ArrowLeft size={18} />
</button>
);
};
and then use it here as
<GoBackButton
action={action}
onClick={transactionId ? closeSidebarClick : undefined}
/>
{ | ||
replace: true, | ||
}, |
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.
Are we sure this doesn't have any unintended side-effects?
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.
At the moment, after testing, I haven't noticed anything wrong
feat: back on completed action fix: back button
7f79d1b
to
7bf0153
Compare
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.
Thanks for implementing my suggestions @CzarekDryl 🙌
The back button for the action sidebar works very nice 🥇
Screen.Recording.2025-01-16.at.19.17.22.mov
I did notice an issue with the browser back navigation. If you select a completed action, use the browser back navigation, an empty action sidebar is shown. However, this issue is also present on master
and is unrelated with your changes, so I'll create a separate issue 👍
Screen.Recording.2025-01-16.at.19.23.34.mov
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.
@CzarekDryl Thank you, nice addition, approving!
Description
Testing
Resolves #3973