Skip to content

feat(docgen-sidebar): support new types for pdf template #4508

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.

Sign up for GitHub

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

Closed
myu-box wants to merge 1 commit into from
+186 −58
Closed

feat(docgen-sidebar): support new types for pdf template #4508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
131 changes: 76 additions & 55 deletions src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useState, useCallback, useEffect } from 'react';
import classNames from 'classnames';
import flow from 'lodash/flow';
import { useIntl } from 'react-intl';
import { useIntl, type MessageDescriptor } from 'react-intl';

import { LoadingIndicator } from '@box/blueprint-web';

Expand Down Expand Up @@ -31,10 +31,77 @@ import { WithLoggerProps } from '../../../common/types/logging';
import commonMessages from '../../common/messages';

import './DocGenSidebar.scss';
import { DocGenTag, DocGenTemplateTagsResponse, JsonPathsMap } from './types';
import type { DocGenTag, DocGenTemplateTagsResponse, JsonPathsMap } from './types';
import { PDF_FIELD_TAG_TYPES, isPdfFormFieldTagType } from './types';

const DEFAULT_RETRIES = 10;

type DocGenSection = {
id: string;
message: MessageDescriptor;
tree: JsonPathsMap;
};

const PDF_FIELD_TYPE_MESSAGES: Record<(typeof PDF_FIELD_TAG_TYPES)[number], MessageDescriptor> = {
checkbox: messages.checkboxTags,
radiobutton: messages.radiobuttonTags,
dropdown: messages.dropdownTags,
};

const createNestedObject = (base: JsonPathsMap, paths: string[]) => {
Copy link
Copy Markdown
Collaborator

@jpan-box jpan-box Apr 22, 2026

Choose a reason for hiding this comment

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

naming nitpick: this function is called createNestedObject, but as I understand it .reduce doesn't so much create a new object as much as it modifies an existing one.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce

I feel like something like populateNestedPaths or addNestedPaths would better communicate the mutation to future readers. What do you think?

Copy link
Copy Markdown
Contributor Author

@myu-box myu-box Apr 22, 2026

Choose a reason for hiding this comment

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

combined these logic into one buildJsonPathTree function

src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.tsx

paths.reduce((obj, path) => {
if (!obj[path]) obj[path] = {};
return obj[path];
}, base);
};
const tagsToJsonPaths = (docGenTags: DocGenTag[]): JsonPathsMap => {
Copy link
Copy Markdown
Collaborator

@jpan-box jpan-box Apr 22, 2026

Choose a reason for hiding this comment

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

wondering if this function will need to be used as a util in a future pr? ie should this be factored out into a utils file?

Copy link
Copy Markdown
Contributor Author

@myu-box myu-box Apr 22, 2026

Choose a reason for hiding this comment

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

combined these logic into one buildJsonPathTree function

src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.tsx

const jsonPathsMap: JsonPathsMap = {};

docGenTags.forEach(tag => {
tag.json_paths.forEach(jsonPath => {
const paths = jsonPath.split('.');
createNestedObject(jsonPathsMap, paths);
});
});

return jsonPathsMap;
};

const buildDocGenSections = (data: DocGenTag[]): DocGenSection[] => {
const result: DocGenSection[] = [];
const imageTags = data.filter(tag => tag.tag_type === 'image');
const textTags = data.filter(tag => tag.tag_type !== 'image' && !isPdfFormFieldTagType(tag.tag_type));

if (textTags.length > 0) {
result.push({
id: 'text',
message: messages.textTags,
tree: tagsToJsonPaths(textTags),
});
}

if (imageTags.length > 0) {
result.push({
id: 'image',
message: messages.imageTags,
tree: tagsToJsonPaths(imageTags),
});
}

PDF_FIELD_TAG_TYPES.forEach(fieldType => {
const fieldTags = data.filter(tag => tag.tag_type === fieldType);
if (fieldTags.length > 0) {
result.push({
id: fieldType,
message: PDF_FIELD_TYPE_MESSAGES[fieldType],
tree: tagsToJsonPaths(fieldTags),
});
}
});

return result;
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

type ExternalProps = {
enabled: boolean;
getDocGenTags: () => Promise<DocGenTemplateTagsResponse>;
Expand All @@ -44,49 +111,12 @@ type ExternalProps = {

type Props = ExternalProps & ErrorContextProps & WithLoggerProps;

type TagState = {
text: DocGenTag[];
image: DocGenTag[];
};

type JsonPathsState = {
textTree: JsonPathsMap;
imageTree: JsonPathsMap;
};

const DocGenSidebar = ({ getDocGenTags }: Props) => {
const { formatMessage } = useIntl();

const [hasError, setHasError] = useState<boolean>(false);
const [isLoading, setIsLoading] = useState<boolean>(false);
const [tags, setTags] = useState<TagState>({
text: [],
image: [],
});
const [jsonPaths, setJsonPaths] = useState<JsonPathsState>({
textTree: {},
imageTree: {},
});

const createNestedObject = (base: JsonPathsMap, paths: string[]) => {
paths.reduce((obj, path) => {
if (!obj[path]) obj[path] = {};
return obj[path];
}, base);
};

const tagsToJsonPaths = useCallback((docGenTags: DocGenTag[]): JsonPathsMap => {
const jsonPathsMap: JsonPathsMap = {};

docGenTags.forEach(tag => {
tag.json_paths.forEach(jsonPath => {
const paths = jsonPath.split('.');
createNestedObject(jsonPathsMap, paths);
});
});

return jsonPathsMap;
}, []);
const [sections, setSections] = useState<DocGenSection[]>([]);

const loadTags = useCallback(
async (attempts = DEFAULT_RETRIES) => {
Expand All @@ -101,17 +131,7 @@ const DocGenSidebar = ({ getDocGenTags }: Props) => {
loadTags.call(this, attempts - 1);
} else if (response?.data) {
const { data } = response;
// anything that is not an image tag for this view is treated as a text tag
Copy link
Copy Markdown
Collaborator

@jpan-box jpan-box Apr 22, 2026

Choose a reason for hiding this comment

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

is this comment still helpful to keep?

Copy link
Copy Markdown
Contributor Author

@myu-box myu-box Apr 22, 2026

Choose a reason for hiding this comment

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

thanks for reminder! I moved to this line

src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.tsx

Screenshot 2026-04-22 at 23 18 46

const textTags = data?.filter(tag => tag.tag_type !== 'image') || [];
const imageTags = data?.filter(tag => tag.tag_type === 'image') || [];
setTags({
text: textTags,
image: imageTags,
});
setJsonPaths({
textTree: tagsToJsonPaths(textTags),
imageTree: tagsToJsonPaths(imageTags),
});
setSections(buildDocGenSections(data));
setHasError(false);
setIsLoading(false);
} else {
Expand All @@ -125,14 +145,14 @@ const DocGenSidebar = ({ getDocGenTags }: Props) => {
},
// disabling eslint because the getDocGenTags prop is changing very frequently
// eslint-disable-next-line react-hooks/exhaustive-deps
[tagsToJsonPaths],
[],
);

useEffect(() => {
loadTags(DEFAULT_RETRIES);
}, [loadTags]);

const isEmpty = tags.image.length + tags.text.length === 0;
const isEmpty = sections.length === 0;

return (
<SidebarContent sidebarView={SIDEBAR_VIEW_DOCGEN} title={formatMessage(messages.docGenTags)}>
Expand All @@ -147,8 +167,9 @@ const DocGenSidebar = ({ getDocGenTags }: Props) => {
{!hasError && !isLoading && isEmpty && <EmptyTags />}
{!hasError && !isLoading && !isEmpty && (
<>
<TagsSection message={messages.textTags} data={jsonPaths.textTree} />
<TagsSection message={messages.imageTags} data={jsonPaths.imageTree} />
{sections.map(section => (
<TagsSection key={section.id} message={section.message} data={section.tree} />
))}
</>
)}
</div>
Expand Down
15 changes: 15 additions & 0 deletions src/elements/content-sidebar/DocGenSidebar/messages.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,21 @@ const messages = defineMessages({
description: 'Image tags section header',
defaultMessage: 'Image tags',
},
checkboxTags: {
id: 'be.docGenSidebar.checkboxTags',
description: 'Checkbox tags section header',
defaultMessage: 'Checkbox tags',
},
radiobuttonTags: {
id: 'be.docGenSidebar.radiobuttonTags',
description: 'Radiobutton tags section header',
defaultMessage: 'Radiobutton tags',
},
dropdownTags: {
id: 'be.docGenSidebar.dropdownTags',
description: 'Dropdown tags section header',
defaultMessage: 'Dropdown tags',
},
docGenTags: {
id: 'be.docGenSidebar.docGenTags',
description: 'DocGen sidebar header',
Expand Down
31 changes: 30 additions & 1 deletion src/elements/content-sidebar/DocGenSidebar/stories/DocGenSidebar.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { HttpHandler } from 'msw';
import type { Meta } from '@storybook/react';
import ContentSidebar from '../../ContentSidebar';
import { mockFileRequest } from '../../stories/__mocks__/ContentSidebarMocks';
import mockDocGenTags from '../../__mocks__/DocGenSidebar.mock';
import mockDocGenTags, { mockPdfTemplateData } from '../../__mocks__/DocGenSidebar.mock';

const defaultArgs = {
detailsSidebarProps: {
Expand Down Expand Up @@ -33,6 +33,16 @@ const docGenSidebarProps = {
}),
};

const docGenSidebarPdfTemplateProps = {
enabled: true,
isDocGenTemplate: true,
checkDocGenTemplate: noop,
getDocGenTags: async () => ({
pagination: {},
data: mockPdfTemplateData,
}),
};

export const basic = {
args: {
defaultView: 'docgen',
Expand All @@ -52,6 +62,25 @@ export const withModernizedBlueprint = {
},
};

export const pdfTemplate = {
args: {
defaultView: 'docgen',
docGenSidebarProps: docGenSidebarPdfTemplateProps,
},
};

export const pdfTemplateWithModernizedBlueprint = {
args: {
enableModernizedComponents: true,
defaultView: 'docgen',
docGenSidebarProps: docGenSidebarPdfTemplateProps,
features: {
...global.FEATURE_FLAGS,
previewModernization: { enabled: true },
},
},
};

const meta: Meta<typeof ContentSidebar> & { parameters: { msw: { handlers: HttpHandler[] } } } = {
title: 'Elements/ContentSidebar/DocGenSidebar',
component: ContentSidebar,
Expand Down
18 changes: 17 additions & 1 deletion src/elements/content-sidebar/DocGenSidebar/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
// our apis are in snake case
export type DocGenTag = {
/* eslint-disable-next-line camelcase */
tag_type: 'text' | 'arithmetic' | 'conditional' | 'for-loop' | 'table-loop' | 'image';
tag_type:
| 'text'
| 'arithmetic'
| 'conditional'
| 'for-loop'
| 'table-loop'
| 'image'
| 'checkbox'
| 'radiobutton'
| 'dropdown';
/* eslint-disable-next-line camelcase */
tag_content: string;
/* eslint-disable-next-line camelcase */
Expand All @@ -20,3 +29,10 @@ export type DocGenTemplateTagsResponse = {
export interface JsonPathsMap {
[key: string]: JsonPathsMap | {};
}

/** PDF template control tags that render in their own sidebar section. */
export const PDF_FIELD_TAG_TYPES = ['checkbox', 'radiobutton', 'dropdown'] as const;

export function isPdfFormFieldTagType(tagType: DocGenTag['tag_type']): boolean {
return (PDF_FIELD_TAG_TYPES as readonly DocGenTag['tag_type'][]).includes(tagType);
}
28 changes: 28 additions & 0 deletions src/elements/content-sidebar/__mocks__/DocGenSidebar.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,32 @@ const mockData = [
},
];

/** PDF template tags (text, checkbox, radiobutton, dropdown) */
export const mockPdfTemplateData = [
{
tag_type: 'text',
tag_content: '{{NameField::optional}}',
json_paths: ['NameField'],
required: false,
Copy link
Copy Markdown
Collaborator

@jpan-box jpan-box Apr 22, 2026

Choose a reason for hiding this comment

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

I notice all the other objects in this page have tag_type, tag_content, and json_paths. where does required come from?

I am guessing the type for this object is defined here: https://github.com/box/box-ui-elements/blob/master/src/elements/content-sidebar/DocGenSidebar/types.ts#L2

which also does not have required. just want to make sure the types match up correctly here

Copy link
Copy Markdown
Contributor Author

@myu-box myu-box Apr 22, 2026

Choose a reason for hiding this comment

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

Good catch!! The property already exists but was missing in type.ts. Updated.

https://github.com/box/box-ui-elements/pull/4513/changes#diff-d401466becb8a7d1db5a1746e7c8a036961aaaf8b4a8d0abe97e27277b932319

},
{
tag_type: 'checkbox',
tag_content: '{{SubscribeCheckbox}}',
json_paths: ['SubscribeCheckbox'],
required: true,
},
{
tag_type: 'radiobutton',
tag_content: '{{Gender}}',
json_paths: ['Gender'],
required: true,
},
{
tag_type: 'dropdown',
tag_content: '{{CountryDropdown}}',
json_paths: ['CountryDropdown'],
required: true,
},
];

export default mockData;
21 changes: 20 additions & 1 deletion src/elements/content-sidebar/__tests__/DocGenSidebar.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { render, screen, waitFor, fireEvent } from '../../../test-utils/testing-library';

import { DocGenSidebarComponent as DocGenSidebar } from '../DocGenSidebar/DocGenSidebar';
import mockData from '../__mocks__/DocGenSidebar.mock';
import mockData, { mockPdfTemplateData } from '../__mocks__/DocGenSidebar.mock';

const docGenSidebarProps = {
getDocGenTags: jest.fn().mockReturnValue(
Expand Down Expand Up @@ -60,6 +60,25 @@ describe('elements/content-sidebar/DocGenSidebar', () => {
expect(tagList).toHaveLength(2);
});

test('should render PDF template tags in separate sections', async () => {
renderComponent({
getDocGenTags: jest.fn().mockReturnValue(
Promise.resolve({
pagination: {},
data: mockPdfTemplateData,
}),
),
});

const tagList = await screen.findAllByTestId('bcs-TagsSection');
Copy link
Copy Markdown
Collaborator

@jpan-box jpan-box Apr 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

is it completely necessary to use testIds? our general practice is to use semantic queries when possible like getByRole. just want to double check here - trusting your judgement.

Copy link
Copy Markdown
Contributor Author

@myu-box myu-box Apr 22, 2026

Choose a reason for hiding this comment

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

Correct. I followed the existing test style at first, but the behavior we care about is the rendered labels, so findByText (and getByRole when it applies) fit better than testid.

https://github.com/box/box-ui-elements/pull/4513/changes#diff-2d5f346ca4d1e1ac53d9b83c2f0a69cfcd5ab5cda77a5935932897048d3072e3

I removed the toHaveLength(4) . Besides, also added a new test that an unknown tag_type appears under text tag section.

expect(tagList).toHaveLength(4);
Copy link
Copy Markdown
Collaborator

@jpan-box jpan-box Apr 22, 2026

Choose a reason for hiding this comment

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

I don't know how important this is, but since you have checked to see if there is a length of 4 with specific mock data, are there cases where it should return 3,2,1, or even 0 or invalid tag types? I have little to no context on what the api shape is like, i'm only thinking from a testing depth perspective. maybe some sort of parameterized test to cover all the cases.

Copy link
Copy Markdown
Contributor Author

@myu-box myu-box Apr 22, 2026

Choose a reason for hiding this comment

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

Correct. I followed the existing test style at first, but the behavior we care about is the rendered labels, so findByText (and getByRole when it applies) fit better than testid.

https://github.com/box/box-ui-elements/pull/4513/changes#diff-2d5f346ca4d1e1ac53d9b83c2f0a69cfcd5ab5cda77a5935932897048d3072e3

I removed the toHaveLength(4) . Besides, also added a new test that an unknown tag_type appears under text tag section.


expect(await screen.findByText('Text tags')).toBeInTheDocument();
expect(screen.getByText('Checkbox tags')).toBeInTheDocument();
expect(screen.getByText('Radiobutton tags')).toBeInTheDocument();
expect(screen.getByText('Dropdown tags')).toBeInTheDocument();
});

test('should render DocGen sidebar component correctly with tags list', async () => {
renderComponent();
const parentTag = await screen.findByText('about');
Expand Down
Loading