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

Poc/code connect #8068

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Poc/code connect #8068

wants to merge 3 commits into from

Conversation

weronikaolejniczak
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak commented Oct 9, 2024

Code Connect

Auto-generated code snippets with mapped properties between Figma and code.

Table of content

Scope

The purpose of this investigation was to:

  • see how Figma's Dev Mode Code Connect integrates with EUI to improve design-to-code workflows,
  • define the most used components to be integrated as the first iteration,
  • identify potential technical challenges, compatibility issues, and benefits of this integration,
  • provide a recommendation on whether or not to proceed with using Code Connect for EUI projects.

For the purpose of the investigation, I used the EUI usage metrics to define the most common components (this excludes Cloud UI) which provide enough complexity:

  1. EuiButton (1,606), EuiButtonEmpty (1,274)
  2. EuiButtonGroup (122)
  3. EuiPanel (1,050)
  4. EuiToolTip (1,140)
  5. EuiCallOut (990)
  6. EuiBadge (605)
  7. EuiFieldText (454)
  8. EuiComboBox (314)
  9. EuiSelect (304)

There are some components that are used often but are less complex and their auto-generated code snippets might not be as useful for investigation purposes:

  • EuiSpacer (5,399)
  • EuiHorizontalRule (480)
  • EuiText (3,364), EuiTitle (1,549), EuiLink (1,549)
  • EuiIcon (1,241), EuiButtonIcon (785)

Summary

It is technically feasible to integrate Code Connect. It will significantly speed up feature development, onboard new developers quicker, minimize confusion regarding component usage, provide the space to advise best practices and highlight all discrepancies between the design and code implementation.

On the other hand, the integration will depend on the designers using the official main components and not detaching instances, and it will be fragile to any big (like the Visual Refresh initiative) or small (renaming a Figma property) Figma component library changes and a chore to set up and maintain (for the EUI development team).

Most of the tricky use cases can be handled without much issue (e.g. a variant in Figma being a separate component in the code) and some, while possible, require a lot of work (e.g. integrating icons). Storybook integration in its current form is poorly configurable, creates conflicts between Figma property mapping, React component props and story arguments, and has bugs (e.g. code snippet of nested components not hydrated). It would be recommended to use standalone Figma files with greater flexibility at the cost of more maintenance.

To bring benefit to all front-end / full-stack engineers, Dev Mode seats have to be provided to all.

Considerations

  • Requires a Dev Mode seat to use. Dev Mode is under investigation and if it does not prove to be of high value compared to its price, we might not end up buying the licenses, rendering Code Connect unusable.
  • We are working on a new component library file in Figma. Therefore, we cannot reap the full benefits of Code Connect straight away. Why?
    • If we decide to integrate the current EUI library, we will make a chore for components that will become deprecated / removed once the Visual Refresh initiative ends and designers start using the new library
    • If we decide to integrate the new EUI library, we might lose component instance links for most of the library, i.e. code snippets will not be propagated across all product designs. Only the new designs that use the new library components will have auto-generated snippets.
  • With the current icons and typography setup, and the on-going cleanup initiative, it will be hard to integrate them with Code Connect in an effective and maintainable way. More on this below in the Typography and Icons sections. Both are topics to be tackled in more detail once the decision on Dev Mode and Code Connect is made.
  • "Code Connect files are not executed. While they're written using real components from your codebase, the Figma CLI essentially treats code snippets as strings. This means you can use, for example, hooks without needing to mock data. However, this also means that logical operators such as ternaries or conditionals will be output verbatim in your example code rather than executed to show the result. You also won't be able to dynamically construct figma.connect calls in a for-loop, as an example." source: https://github.com/figma/code-connect/blob/main/docs/react.md#basic-setup

Demo

Short video demo here.
Figma file here.

Note: For most of these cases, the integration could be even better.

Conclusions

Benefits

  • As shown in the demo above, developers can look for how to implement a component instance in the EUI documentation, in the Storybook, in the component source code or in the existing feature implementation. All of those options require experience with the library and / or time and focus. Auto-generated code snippets significantly reduce the time to develop a functionality because they're readily available. They may also provide a head-start to onboarding developers.
  • As shown by the Component Insights section, Code Connect can be a great exercise to evaluate and assert the parity between Figma design and code implementation.
  • The code snippets can showcase usage with companion hooks, encourage best practices and influence code quality, and overall application accessibility (e.g. using aria-label when "Icon only" property is toggled).

Limitations

  • Dedicated integration files (*.figma.tsx) are another file to maintain while keeping the Figma integration in Storybook files means less configuration capabilities (at the moment) and busier stories files.
  • Resolving any type errors that do not cause runtime issues so can be "ignored" are tricky to resolve. TypeScript expressions like type assertion (as Y, e.g. as const to narrow down the type) and @ts-ignore or @ts-expect-error comment annotations will be shown in the code snippet because the examples are not executed but resolved as a string.
  • Figma doesn't always rehydrate code snippets with other component instances. Sometimes, even when a component is integrated, it will display the purple badge instead of that component's code snippet (see EuiPanel).
  • For Storybook, component instances are not populated at all, i.e. if a component is used in an instance swap or as children, and is integrated, the parent component still displays the purple badge instead of component's code snippet (see EuiPanel).
  • Code Connect doesn't consider Prettier and ESLint configuration, e.g. semicolons are missing. semicolons stripped in React figma/code-connect#152
  • There are no usage analytics implemented yet. Usage Tracking and Analytics for Code-Connected Components figma/code-connect#144
  • Whenever there are some nested components, several imports will be added. Right now, there's no possibility to merge them into one import statement. Single import statment with multiple components :: Nested instances figma/code-connect#69
  • There's no possibility to connect multiple stories to multiple components in Figma (story-level connection). Connecting multiple stories in one file to multiple Figma components figma/code-connect#35

Storybook

  • Requires a "render" function to be defined. Most component stories do not define it leading to additional chore to be made.
// component.stories.tsx
  
export const Playground: Story = {
	render: ({ children, ...props }) => (
		<EuiCallOut {...props}>{children}</EuiCallOut>
	),
};
  • Requires a separate "parameters" config object with a props mapping, regardless of explicit or implicit controls. Furthermore, there can be annoying discrepancies between Figma properties (in the code), component props and Storybook args.
// component.stories.tsx

export const Playground = () => {
	// 3. Notice how here we get:
	// "ariaLabel" from Code Connect mapping (it can only be camelCase), and
	// "aria-label" from Storybook args (either inferred or explicitly defined)
	render: ({ ariaLabel, label, ...args }) => (
		<EuiComboBox aria-label={ariaLabel} label={label} {...args} />
	),
	// SOLUTION: We have to rename Storybook arg to "ariaLabel"
	// and we lose the 1:1 mapping of props to Storybook args.
};

const meta = {
	argTypes: {
		// control types
		'aria-label': { control: 'text' }, // This would become "ariaLabel".
		label: { control: 'text' },
		...
	},
	args: {
		// default values
		'aria-label': 'Aria Label', // This would become "ariaLabel".
		label: 'Label', // 2. Notice how in SB the label is a string value!
		...
	},
	parameters: {
		design: {
			type: 'figma',
			// We cannot leverage URL replacement from global config
			// as in the standalone configuration (*.figma.tsx files).
			url: 'https://www.figma.com/design/{project}/{file}?node-id={id}',
			examples: [Playground],
			props: {
				// 1. Cannot write `aria-label`, otherwise parsing fails;
				// Notice how "Label" in Figma is a boolean value!
				ariaLabel: figma.boolean('Label', {
					true: undefined,
					false: 'Aria Label',
				}),
				label: figma.boolean('Label', {
					true: 'Label',
					false: undefined
				}),
				...
			}
		},
	},
};

export default meta;
  • Another example of the awkward Storybook integration is EuiButtonGroup (button-group.stories.tsx) which has 2 component wrappers: StatefulEuiButtonGroupSingle and StatefulEuiButtonGroupMulti. In order for these components to display correct names in the code snippet, they have to be renamed to EuiButtonGroup. But both cannot be named that way.
    Additional problem is that for the sake of showcasing in Storybook, the wrappers add state logic. This state logic, abstracted in the wrappers, is not explicitly defined in the render function that Code Connect uses for code snippet generation. Therefore, important props are missing (like onChange and idSelected) from the code snippet:
    "Screenshot 2024-10-09 at 17 09 56"

  • If the original component is renamed within the story or a wrapper is created and used for the render functions, there's no way to change it for the code snippet. The only way to mitigate this is to rename the original component when importing and name the wrapper as the original component.

// Current stories code:
const StatefulComboBox = (props: EuiComboBoxProps<{}>) => {
	...
}

// Code snippet:
<StatefulComboBox
	aria-label="Meaningful label"
	compressed
/>

// Solution:
import { EuiComboBox as ComboBox } from './combo_box';

const EuiComboBox = (props: EuiComboBoxProps<{}>) => {
	...
}
  • Storybook Designs add-on configuration capabilities are poor, e.g. we cannot add an import import { Component } from '@elastic/eui'; (there's an issue open for this; on the other hand, developers can rely on auto-import) or resolve the "url" based on global configuration to avoid repetition (on the other hand, there can be a util function that returns the whole Figma URL or a constant to use within a template literal but copy-pasting a link from Figma is quick and easy so it might be a redundant abstraction).
    Screenshot 2024-10-08 at 19 28 57
    image

  • Storybook supports variant restrictions. In the case of standalone *.figma.tsx files, all subcomponents can be integrated within the same file. In the case of Storybook integration, usually each component has a separate *.stories.tsx file, i.e. the relationship between Figma components and code components will now be scattered across different stories, even though in Figma it's the same component. Furthermore, reusing property mapping would be tricky. We'd need to extract sharedProps constant to separate file to be reusable by all stories.
    Example: EuiButton and EuiButtonEmpty. Figma's Button style "empty" maps to a separate component in the code - EuiButtonEmpty. That component has a separate stories file. Therefore, from the level of EuiButton stories file we do not know about this subcomponent and may believe that we just haven't implemented the style for "empty". This might as well turn out to not be an issue. Take a look at:

  • I'm only adding "Playground" story to the "examples" set but there can be multiple stories, each representing a different use case (as per Storybook snippet in Variant restrictions):

export default {
  component: Button,
  parameters: {
    design: {
      type: 'figma',
      url: 'https://...',
      examples: [
        { example: PrimaryButtonStory, variant: { Type: 'Primary' } },
        { example: SecondaryButtonStory, variant: { Type: 'Secondary' } },
        { example: DangerButtonStory, variant: { Type: 'Danger' } },
      ],
    },
  },
}

export function PrimaryButtonStory() {
  return <PrimaryButton />
}

export function SecondaryButtonStory() {
  return <SecondaryButton />
}

export function DangerButtonStory() {
  return <DangerButton />
}

Typography

Typography integration depends on whether the Figma library uses text styles (optionally with typography variables) or a component with instance swap. In the latter case, integration is possible because code implementation is based on the usage of typography components (e.g. EuiText and EuiTitle). But most of the time, the former is true because it's easier to manage by the designers.

Whatever the case, the EUI Figma library will support both ways. We can integrate typography components easily and display accurate code snippets for them. We cannot map in any way text styles.

For an example, see EuiPanel integration.

There's an issue open for this topic in Code Connect GH repo.

Icons

Each icon (glyph) is a separate component in Figma. For the purposes of this explanation, let's call them atomic icon components. They are used as an instance swap in all components, including an Icon component which is used as a standalone icon. This provides a unique challenge because most EUI components in the code, e.g. EuiButton, receive an icon name / source / SVG element in the iconType prop, and not the EuiIcon component instance:

// 🟢 It's this:
<EuiButton iconType="accessibility">Hello</EuiButton>

// 🔴 and NOT this:
<EuiButton icon={<EuiIcon type="accessibility" />}>Hello</EuiButton>

and that's the reverse approach to the Figma component library:
Screenshot 2024-10-08 at 14 20 09
Screenshot 2024-10-08 at 14 18 57

which results in a weird-ish code snippet being generated when figma.instance is used (no better alternative):
Screenshot 2024-10-08 at 14 36 20

where this "Icon" is a Figma component reference. Upon copying the code snippet to the clipboard, here's what we get:

<EuiButton
    onClick={() => {}}
    color="primary"
    iconType={/* Icon right */} // this isn't the icon we set, we have to manually change it to desired icon name
    iconSide="right"
    size="m"
>
    Add element
</EuiButton>

If I connected the chosen icon component (say "accessibility") in Figma to EuiIcon component, then Figma would hydrate the EuiButton code snippet with the EuiIcon code snippet of the accessibility icon instance swap, resulting in:

<EuiButton icon={<EuiIcon type="accessibility" />}>Hello</EuiButton>

Circling to the beginning, that's not how the Button is designed to be used (with render props).

The Icon component is easily integrated because it's used as a standalone component and not an instance swap:

Screenshot 2024-10-08 at 14 24 23

On the other hand, the atomic icon components are used as swappable instances in its "Type" property. In theory, those should be mapped to a string, resulting in the following code snippet:

import { EuiIcon } from "@elastic/eui"

<EuiIcon
    type="accessibility"
    size="m"
/>

Unfortunately, this is not possible and will result in "accessibility" being a comment upon copying and pasting because it's treated as an unlinked React component:

<EuiIcon
    type={/* accessibility */}
    size="m"
/>

One solution would be to, instead of integrating atomic icon components, integrate all icon instance swaps with the EuiButton. That is not sustainable in the long run. Imagine integrating all components that consume an icon the number of times there are icons in the library.

It is recommended to create a script that pulls icons from a Figma file and generates icons.figma.tsx with appropriate integrations. You can read more about it here.

There are some issues on the topic in Code Connect GH repo:

Component Insights

EuiButton, EuiButtonEmpty

Figma | Docs | Code

Screen.Recording.2024-10-11.at.14.21.41.mov
  • The size "Extra Small" is not implemented in the component library.
  • The style "Empty" is a separate component - EuiButtonEmpty - seemingly the same, worth investigating if it can be merged.

EuiButtonGroup

Figma | Docs | Code

Screen.Recording.2024-10-11.at.14.17.14.mov
  • In Figma, there are no styles: "Accent" | "Success" | "Warning" | "Danger" but they are implemented in code.

EuiPanel

Figma | Docs | Code

Screen.Recording.2024-10-11.at.14.25.37.mov
  • Because false values are resolved to no prop passed, we cannot replicate hasShadow={true} interface in the integration. A workaround are variant restrictions (as explained here), unfortunately Code Connect throws an error: "The Figma Variant "Shadow" does not have an option for true". For now, I skipped hasShadow prop altogether.

EuiBadge

Figma | Docs | Code

Screen.Recording.2024-10-10.at.10.52.54.mov
  • In Figma, we can define both the left and the right icon at the same time. In code, there are component props iconType and iconSide, and we can only ever have one icon at a time.

EuiCallOut

Figma | Docs | Code

Screen.Recording.2024-10-11.at.13.00.55.mov
  • Primary and secondary buttons in the content are not controlled through the component. Furthermore, there is no story or a documentation entry that explains how to achieve that result.

EuiToolTip

Figma | Docs | Code

Screen.Recording.2024-10-11.at.13.05.07.mov

EuiFieldText

Figma | Docs | Code

Screen.Recording.2024-10-11.at.13.34.08.mov
  • It wasn't clear to me how to map "Column display". It doesn't seem to be controlled by the EuiFieldText or EuiFormRow (unless withdisplay prop).

EuiComboBox

Figma | Docs | Code

Screen.Recording.2024-10-11.at.13.49.05.mov
  • It wasn't clear to me how to map "Column display". It doesn't seem to be controlled by the EuiComboBox or EuiFormRow (unless withdisplay prop).

EuiSelect

Figma | Docs | Code

Screen.Recording.2024-10-11.at.13.54.09.mov
  • It wasn't clear to me how to map "Column display". It doesn't seem to be controlled by the EuiSelect or EuiFormRow (unless withdisplay prop).

Sources

@weronikaolejniczak weronikaolejniczak force-pushed the poc/code-connect branch 2 times, most recently from 8a1899e to 5a942db Compare October 11, 2024 12:00
@weronikaolejniczak weronikaolejniczak force-pushed the poc/code-connect branch 2 times, most recently from 37e2b60 to a299dbe Compare October 11, 2024 12:26
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants