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

fix(content-sidebar): remove dependency on isSignRemoveInterstitialEnabled #3734

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,6 @@ be.contentSidebar.boxSignRequestSignature = Request Signature
be.contentSidebar.boxSignSecurityBlockedTooltip = This action is unavailable due to a security policy.
# One of the dropdown options that opens a Box Sign sign myself experience
be.contentSidebar.boxSignSignMyself = Sign Myself
# label for button that opens a Box Sign signature fulfillment experience
be.contentSidebar.boxSignSignature = Sign
# Tooltip text for when Box Sign is blocked due to an item being watermarked
be.contentSidebar.boxSignWatermarkBlockedTooltip = This action is unavailable, because the file is watermarked.
# title for when editing an existing approval task
Expand Down
4 changes: 4 additions & 0 deletions src/elements/content-sidebar/ContentSidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type { DetailsSidebarProps } from './DetailsSidebar';
import type { DocGenSidebarProps } from './DocGenSidebar/DocGenSidebar';
import type { MetadataSidebarProps } from './MetadataSidebar';
import type { VersionsSidebarProps } from './versions';
import type { SignSideBarProps } from './SidebarNavSign';
import type { WithLoggerProps } from '../../common/types/logging';
import type { ElementsXhrError, RequestOptions, ErrorContextProps } from '../../common/types/api';
import type { MetadataEditor } from '../../common/types/metadata';
Expand Down Expand Up @@ -87,6 +88,7 @@ type Props = {
sharedLink?: string,
sharedLinkPassword?: string,
theme?: Theme,
signSidebarProps: SignSideBarProps,
token: Token,
versionsSidebarProps: VersionsSidebarProps,
} & ErrorContextProps &
Expand Down Expand Up @@ -368,6 +370,7 @@ class ContentSidebar extends React.Component<Props, State> {
onVersionHistoryClick,
theme,
versionsSidebarProps,
signSidebarProps,
}: Props = this.props;
const { file, isLoading, metadataEditors }: State = this.state;
const initialPath = defaultView.charAt(0) === '/' ? defaultView : `/${defaultView}`;
Expand Down Expand Up @@ -410,6 +413,7 @@ class ContentSidebar extends React.Component<Props, State> {
onVersionHistoryClick={onVersionHistoryClick}
theme={theme}
versionsSidebarProps={versionsSidebarProps}
signSidebarProps={signSidebarProps}
wrappedComponentRef={ref => {
this.sidebarRef = ref;
}}
Expand Down
4 changes: 4 additions & 0 deletions src/elements/content-sidebar/Sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import type { VersionsSidebarProps } from './versions';
import type { AdditionalSidebarTab } from './flowTypes';
import type { MetadataEditor } from '../../common/types/metadata';
import type { BoxItem, User } from '../../common/types/core';
import type { SignSideBarProps } from './SidebarNavSign';
import type { Errors } from '../common/flowTypes';
// $FlowFixMe TypeScript file
import type { Theme } from '../common/theming';
Expand Down Expand Up @@ -70,6 +71,7 @@ type Props = {
onVersionChange?: Function,
onVersionHistoryClick?: Function,
theme?: Theme,
signSidebarProps: SignSideBarProps,
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this is alphabetize, can we alphabetize this prop too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

versionsSidebarProps: VersionsSidebarProps,
};

Expand Down Expand Up @@ -311,6 +313,7 @@ class Sidebar extends React.Component<Props, State> {
onVersionChange,
theme,
versionsSidebarProps,
signSidebarProps,
}: Props = this.props;
const isOpen = this.isOpen();
const hasBoxAI = SidebarUtils.canHaveBoxAISidebar(this.props);
Expand Down Expand Up @@ -346,6 +349,7 @@ class Sidebar extends React.Component<Props, State> {
hasMetadata={hasMetadata}
hasSkills={hasSkills}
hasDocGen={docGenSidebarProps.isDocGenTemplate}
signSideBarProps={signSidebarProps}
isOpen={isOpen}
onPanelChange={this.handlePanelChange}
/>
Expand Down
7 changes: 5 additions & 2 deletions src/elements/content-sidebar/SidebarNav.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
import { useFeatureConfig } from '../common/feature-checking';
import type { NavigateOptions, AdditionalSidebarTab } from './flowTypes';
import './SidebarNav.scss';
import type { SignSideBarProps } from './SidebarNavSign';

type Props = {
additionalTabs?: Array<AdditionalSidebarTab>,
Expand All @@ -49,6 +50,7 @@ type Props = {
isOpen?: boolean,
onNavigate?: (SyntheticEvent<>, NavigateOptions) => void,
onPanelChange?: (name: string, isInitialState: boolean) => void,
signSideBarProps: SignSideBarProps,
};

const SidebarNav = ({
Expand All @@ -66,8 +68,9 @@ const SidebarNav = ({
isOpen,
onNavigate,
onPanelChange = noop,
signSideBarProps,
}: Props) => {
const { enabled: hasBoxSign } = useFeatureConfig('boxSign');
const { enabled: hasBoxSign } = signSideBarProps || {};
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 a permission via the user that enables this feature too ?

Copy link
Contributor Author

@diogostavares diogostavares Jan 17, 2025

Choose a reason for hiding this comment

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

Yes, there is a user permission controlled by the admin that enables/disables signing. This is reflected in the enabled property sent by EUA

const { disabledTooltip: boxAIDisabledTooltip, showOnlyNavButton: showOnlyBoxAINavButton } =
useFeatureConfig('boxai.sidebar');

Expand Down Expand Up @@ -159,7 +162,7 @@ const SidebarNav = ({

{hasBoxSign && (
<div className="bcs-SidebarNav-secondary">
<SidebarNavSign />
<SidebarNavSign {...signSideBarProps} />
</div>
)}

Expand Down
65 changes: 29 additions & 36 deletions src/elements/content-sidebar/SidebarNavSign.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import * as React from 'react';
import { FormattedMessage } from 'react-intl';

// @ts-ignore Module is written in Flow
import { useFeatureConfig } from '../common/feature-checking';
// @ts-ignore Module is written in Flow
import { SIDEBAR_NAV_TARGETS } from '../common/interactionTargets';

Expand All @@ -17,48 +15,43 @@ import { Menu, MenuItem } from '../../components/menu';
import messages from './messages';

import './SidebarNavSign.scss';
// @ts-ignore Module is written in Flow
import type { TargetingApi } from '../../features/targeting/types';

export interface SignSideBarProps {
blockedReason: string;
enabled: boolean;
onClick: () => void;
onClickSignMyself: () => void;
targetingApi: TargetingApi | null;
}

export function SidebarNavSign() {
export function SidebarNavSign(signSideBarProps: SignSideBarProps) {
const {
blockedReason: boxSignBlockedReason,
onClick: onBoxClickRequestSignature,
onClickSignMyself: onBoxClickSignMyself,
status: boxSignStatus,
targetingApi: boxSignTargetingApi,
isSignRemoveInterstitialEnabled,
} = useFeatureConfig('boxSign');
} = signSideBarProps;

return (
<>
{isSignRemoveInterstitialEnabled ? (
<DropdownMenu isResponsive constrainToWindow isRightAligned>
<SidebarNavSignButton
blockedReason={boxSignBlockedReason}
status={boxSignStatus}
targetingApi={boxSignTargetingApi}
data-resin-target={SIDEBAR_NAV_TARGETS.SIGN}
/>
<Menu>
<MenuItem data-testid="sign-request-signature-button" onClick={onBoxClickRequestSignature}>
<SignMeOthers32 width={16} height={16} className="bcs-SidebarNavSign-icon" />
<FormattedMessage {...messages.boxSignRequestSignature} />
</MenuItem>
<MenuItem data-testid="sign-sign-myself-button" onClick={onBoxClickSignMyself}>
<SignMe32 width={16} height={16} className="bcs-SidebarNavSign-icon" />
<FormattedMessage {...messages.boxSignSignMyself} />
</MenuItem>
</Menu>
</DropdownMenu>
) : (
<SidebarNavSignButton
blockedReason={boxSignBlockedReason}
data-resin-target={SIDEBAR_NAV_TARGETS.SIGN}
onClick={onBoxClickRequestSignature}
status={boxSignStatus}
targetingApi={boxSignTargetingApi}
/>
)}
</>
<DropdownMenu isResponsive constrainToWindow isRightAligned>
<SidebarNavSignButton
blockedReason={boxSignBlockedReason}
targetingApi={boxSignTargetingApi}
data-resin-target={SIDEBAR_NAV_TARGETS.SIGN}
/>
<Menu>
<MenuItem onClick={onBoxClickRequestSignature}>
<SignMeOthers32 width={16} height={16} className="bcs-SidebarNavSign-icon" />
<FormattedMessage {...messages.boxSignRequestSignature} />
</MenuItem>
<MenuItem onClick={onBoxClickSignMyself}>
<SignMe32 width={16} height={16} className="bcs-SidebarNavSign-icon" />
<FormattedMessage {...messages.boxSignSignMyself} />
</MenuItem>
</Menu>
</DropdownMenu>
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/elements/content-sidebar/SidebarNavSignButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ export type Props = PlainButtonProps & {

export const PlaceholderTooltip = ({ children }: { children: React.ReactNode }) => children;

export function SidebarNavSignButton({ blockedReason, intl, status, targetingApi, ...rest }: Props) {
export function SidebarNavSignButton({ blockedReason, intl, targetingApi, ...rest }: Props) {
const isSignDisabled = !!blockedReason;
const isTargeted = targetingApi && targetingApi.canShow;
const isTargeted = targetingApi?.canShow;
const FtuxTooltip = !isSignDisabled && isTargeted ? TargetedClickThroughGuideTooltip : PlaceholderTooltip;
const label = intl.formatMessage(status === 'active' ? messages.boxSignSignature : messages.boxSignRequest);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this check because status property is not being passed anymore from EUA

const label = intl.formatMessage(messages.boxSignRequest);
const buttonClassName = classnames('bcs-SidebarNavSignButton', { 'bdl-is-disabled': isSignDisabled });

let tooltipMessage = label;
Expand Down
12 changes: 4 additions & 8 deletions src/elements/content-sidebar/__tests__/SidebarNav.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,19 +166,15 @@ describe('elements/content-sidebar/SidebarNav', () => {
});

test('should render the Box Sign entry point if its feature is enabled', () => {
const features = {
boxSign: {
const props = {
signSideBarProps: {
enabled: true,
onClick: () => {},
},
};
const wrapper = getWrapper({}, 'activity', features);
expect(wrapper.exists(SidebarNavSignButton)).toBe(true);
});
const wrapper = getWrapper(props);

test('should not render the Box Sign entry point if its feature is not enabled', () => {
const wrapper = getWrapper();
expect(wrapper.exists(SidebarNavSignButton)).toBe(false);
expect(wrapper.exists(SidebarNavSignButton)).toBe(true);
});
test('should render docgen tab', () => {
const props = {
Expand Down
93 changes: 33 additions & 60 deletions src/elements/content-sidebar/__tests__/SidebarNavSign.test.tsx
Original file line number Diff line number Diff line change
@@ -1,80 +1,53 @@
import * as React from 'react';
import { render, fireEvent } from '@testing-library/react';

import { userEvent } from '@testing-library/user-event';
import { render, screen } from '../../../test-utils/testing-library';
import SidebarNavSign from '../SidebarNavSign';
// @ts-ignore Module is written in Flow
import FeatureProvider from '../../common/feature-checking/FeatureProvider';

describe('elements/content-sidebar/SidebarNavSign', () => {
const onClickRequestSignature = jest.fn();
const onClickSignMyself = jest.fn();

const renderComponent = (props = {}, features = {}) =>
render(
<FeatureProvider features={features}>
<SidebarNavSign {...props} />
</FeatureProvider>,
);
const defaultSignSideBarProps = {
blockedReason: '',
enabled: true,
onClick: onClickRequestSignature,
onClickSignMyself,
targetingApi: null,
};

const renderComponent = () => render(<SidebarNavSign {...defaultSignSideBarProps} />, {});

test.each([true, false])('should render sign button', isRemoveInterstitialEnabled => {
const features = {
boxSign: {
isSignRemoveInterstitialEnabled: isRemoveInterstitialEnabled,
},
};
test('should render sign button', async () => {
renderComponent();

const wrapper = renderComponent({}, features);
expect(wrapper.getByTestId('sign-button')).toBeVisible();
expect(screen.getByRole('button', { name: 'Request Signature' })).toBeInTheDocument();
});

test('should call correct handler when sign button is clicked', () => {
const features = {
boxSign: {
isSignRemoveInterstitialEnabled: false,
onClick: onClickRequestSignature,
},
};
const { getByTestId } = renderComponent({}, features);
test('should open dropdown with 2 menu items when sign button is clicked', async () => {
renderComponent();

fireEvent.click(getByTestId('sign-button'));
await userEvent.click(screen.getByRole('button', { name: 'Request Signature' }));

expect(onClickRequestSignature).toBeCalled();
expect(screen.getByRole('menuitem', { name: 'Request Signature' })).toBeVisible();
expect(screen.getByRole('menuitem', { name: 'Sign Myself' })).toBeVisible();
});

test('should open dropdown with 2 menu items when sign button is clicked', () => {
const features = {
boxSign: {
isSignRemoveInterstitialEnabled: true,
},
};
const { getByTestId } = renderComponent({}, features);
fireEvent.click(getByTestId('sign-button'));
expect(getByTestId('sign-request-signature-button')).toBeVisible();
expect(getByTestId('sign-sign-myself-button')).toBeVisible();
});
test('should call correct handler when request signature option is clicked', async () => {
renderComponent();

test('should call correct handler when request signature option is clicked', () => {
const features = {
boxSign: {
isSignRemoveInterstitialEnabled: true,
onClick: onClickRequestSignature,
},
};
const { getByTestId } = renderComponent({}, features);
fireEvent.click(getByTestId('sign-button'));
fireEvent.click(getByTestId('sign-request-signature-button'));
expect(onClickRequestSignature).toBeCalled();
await userEvent.click(screen.getByRole('button', { name: 'Request Signature' }));
await userEvent.click(screen.getByRole('menuitem', { name: 'Request Signature' }));

expect(onClickRequestSignature).toHaveBeenCalled();
});

test('should call correct handler when sign myself option is clicked', () => {
const features = {
boxSign: {
isSignRemoveInterstitialEnabled: true,
onClickSignMyself,
},
};
const { getByTestId } = renderComponent({}, features);
fireEvent.click(getByTestId('sign-button'));
fireEvent.click(getByTestId('sign-sign-myself-button'));
expect(onClickSignMyself).toBeCalled();
test('should call correct handler when sign myself option is clicked', async () => {
renderComponent();

await userEvent.click(screen.getByRole('button', { name: 'Request Signature' }));
await userEvent.click(screen.getByRole('menuitem', { name: 'Sign Myself' }));

expect(onClickSignMyself).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,11 @@ import Tooltip from '../../../components/tooltip';
describe('elements/content-sidebar/SidebarNavSignButton', () => {
const getWrapper = (props = {}) => shallow(<SidebarNavSignButton {...props} />).dive();

test.each`
status | label
${undefined} | ${'Request Signature'}
${'random'} | ${'Request Signature'}
${'active'} | ${'Sign'}
`('should render the correct label based on the current signature status', ({ label, status }) => {
const wrapper = getWrapper({ status });
test('should render the correct label', () => {
const wrapper = getWrapper();

expect(wrapper.find(Tooltip).prop('text')).toBe(label);
expect(wrapper.find(PlainButton).prop('aria-label')).toBe(label);
expect(wrapper.find(Tooltip).prop('text')).toBe('Request Signature');
expect(wrapper.find(PlainButton).prop('aria-label')).toBe('Request Signature');
expect(wrapper.exists(BoxSign28)).toBe(true);
});

Expand Down
5 changes: 0 additions & 5 deletions src/elements/content-sidebar/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ const messages = defineMessages({
defaultMessage: 'Sign Myself',
description: 'One of the dropdown options that opens a Box Sign sign myself experience',
},
boxSignSignature: {
id: 'be.contentSidebar.boxSignSignature',
defaultMessage: 'Sign',
description: 'label for button that opens a Box Sign signature fulfillment experience',
},
boxSignSecurityBlockedTooltip: {
defaultMessage: 'This action is unavailable due to a security policy.',
description: 'Tooltip text for when Box Sign is blocked due to a security policy',
Expand Down
Loading