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: add back button to action sidebar #4016

Merged
merged 2 commits into from
Jan 18, 2025
Merged

Conversation

CzarekDryl
Copy link
Contributor

@CzarekDryl CzarekDryl commented Dec 30, 2024

Description

  • Added back button to action sidebar
image

Testing

  1. Back Arrow:
  • Appears only after an action is selected.
  • Navigates to the relevant action list view category.
  • Not visible on the main action list view.
  1. Completed Actions:
  • Navigates to the relevant list view category.
  1. Browser Back Button:
  • Returns to the previous action transaction if clicked after navigating away.
  1. Hover State:
  • On desktop, matches other action header icons and uses the blue-400 color.

Resolves #3973

@CzarekDryl CzarekDryl self-assigned this Dec 30, 2024
@CzarekDryl CzarekDryl requested a review from a team as a code owner December 30, 2024 14:32
@CzarekDryl CzarekDryl marked this pull request as draft December 30, 2024 14:47
@CzarekDryl CzarekDryl marked this pull request as ready for review December 31, 2024 08:27
Copy link
Member

@arrenv arrenv left a 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:

working-back-button

@CzarekDryl
Copy link
Contributor Author

@arrenv Browser back button is working fine now

Screen.Recording.2025-01-03.at.11.56.28.mov

@CzarekDryl CzarekDryl requested a review from arrenv January 3, 2025 10:57
Copy link
Contributor

@mmioana mmioana left a 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 🥇

Copy link
Contributor

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;
};

Comment on lines 281 to 222
{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>
)}
Copy link
Contributor

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}
            />

Comment on lines -62 to -65
{
replace: true,
},
Copy link
Contributor

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?

Copy link
Contributor Author

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
@CzarekDryl CzarekDryl force-pushed the feat/16212506-back-button branch from 7f79d1b to 7bf0153 Compare January 15, 2025 15:37
@CzarekDryl CzarekDryl requested a review from mmioana January 15, 2025 15:39
Copy link
Contributor

@mmioana mmioana left a 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

Copy link
Member

@arrenv arrenv left a 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!

@CzarekDryl CzarekDryl merged commit e03b338 into master Jan 18, 2025
2 checks passed
@CzarekDryl CzarekDryl deleted the feat/16212506-back-button branch January 18, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ACTIONS] Add back button to action header
3 participants