ronantakizawa/codereview-bench huggingface dataset:
Repository: storybookjs/storybook
File: code/core/src/manager/settings/GuidePage.tsx (TypeScript)

Code before:

  '& h1': {
    fontSize: theme.typography.size.m3,
    fontWeight: theme.typography.weight.bold,
    margin: 0,
  },

  '& > p': {
    margin: 0,
  },
}));

export const GuidePage = () => {
  const checklist = useChecklist();

  return (
    <Container>
      <Intro>
        <h1>Guide</h1>
        <p>
          Whether you&apos;re just getting started or looking for ways to level up, this checklist
          will help you make the most of your Storybook.
        </p>
      </Intro>
      <Checklist {...checklist} />
      {global.FEATURES?.checklistWidget && (
        <>
          {checklist.openItems.length === 0 ? (
            <center>Your work here is done!</center>
          ) : checklist.widget.disable || checklist.openItems.every((item) => item.isMuted) ? (
            <center>
              Want to see this in the sidebar?{' '}
              <Link onClick={() => checklist.disable(false)}>Show in sidebar</Link>
            </center>
          ) : (
            <center>
              Don&apos;t want to see this in the sidebar?{' '}
              <Link onClick={() => checklist.mute(checklist.allItems.map(({ id }) => id))}>
                Remove from sidebar
              </Link>
            </center>
          )}
        </>
      )}
    </Container>
  );
};


Review comment (suggestion):
Same here, how about
```suggestion
      {global.FEATURES?.checklistWidget !== false && (
```

Code after:

  '& h1': {
    fontSize: theme.typography.size.m3,
    fontWeight: theme.typography.weight.bold,
    margin: 0,
  },

  '& > p': {
    margin: 0,
  },
}));

export const GuidePage = () => {
  const checklist = useChecklist();

  return (
    <Container>
      <Intro>
        <h1>Guide</h1>
        <p>
          Whether you&apos;re just getting started or looking for ways to level up, this checklist
          will help you make the most of your Storybook.
        </p>
      </Intro>
      <Checklist {...checklist} />
      {global.FEATURES?.sidebarOnboardingChecklist !== false && (
        <>
          {checklist.openItems.length === 0 ? (
            <center>Your work here is done!</center>
          ) : checklist.widget.disable || checklist.openItems.every((item) => item.isMuted) ? (
            <center>
              Want to see this in the sidebar?{' '}
              <Link onClick={() => checklist.disable(false)}>Show in sidebar</Link>
            </center>
          ) : (
            <center>
              Don&apos;t want to see this in the sidebar?{' '}
              <Link onClick={() => checklist.mute(checklist.allItems.map(({ id }) => id))}>
                Remove from sidebar
              </Link>
            </center>
          )}
        </>
      )}
    </Container>
  );
};
Repository: storybookjs/storybook
File: code/core/src/types/modules/core-common.ts (TypeScript)

Code before:
     * Enable the integrated controls addon
     *
     * @default true
     */
    controls?: boolean;

    /**
     * Enable the integrated interactions addon
     *
     * @default true
     */
    interactions?: boolean;

    /**
     * Enable the integrated actions addon
     *
     * @default true
     */
    actions?: boolean;

    /**
     * Enable the onboarding checklist sidebar widget
     *
     * @default true
     */
    checklistWidget?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Filter args with a "target" on the type from the render function (EXPERIMENTAL)
     */
    argTypeTargetsV7?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Apply decorators from preview.js before decorators from addons or frameworks
     */
    legacyDecoratorFileOrder?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Disallow implicit actions during rendering. This will be the default in Storybook 8.
     *
     * This will make sure that your story renders the same no matter if docgen is enabled or not.
     */
    disallowImplicitActionsInRenderV8?: boolean;

    /**

Review comment (suggestion):
Could be worth circling back with MA or Kyle on the naming. My assumption is users would search for something "onboarding" related in the docs, so having `onboarding` in the name might help discoverability.

I lean towards `onboardingChecklist` but haven't given this much thought so feel free to discard this opinion unashamedly!

Code after:
     * Enable the integrated controls addon
     *
     * @default true
     */
    controls?: boolean;

    /**
     * Enable the integrated interactions addon
     *
     * @default true
     */
    interactions?: boolean;

    /**
     * Enable the integrated actions addon
     *
     * @default true
     */
    actions?: boolean;

    /**
     * Enable the onboarding checklist sidebar widget
     *
     * @default true
     */
    sidebarOnboardingChecklist?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Filter args with a "target" on the type from the render function (EXPERIMENTAL)
     */
    argTypeTargetsV7?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Apply decorators from preview.js before decorators from addons or frameworks
     */
    legacyDecoratorFileOrder?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Disallow implicit actions during rendering. This will be the default in Storybook 8.
     *
     * This will make sure that your story renders the same no matter if docgen is enabled or not.
     */
    disallowImplicitActionsInRenderV8?: boolean;

    /**
Repository: storybookjs/storybook
File: code/core/src/types/modules/core-common.ts (TypeScript)

Code before:
     * Enable the integrated controls addon
     *
     * @default true
     */
    controls?: boolean;

    /**
     * Enable the integrated interactions addon
     *
     * @default true
     */
    interactions?: boolean;

    /**
     * Enable the integrated actions addon
     *
     * @default true
     */
    actions?: boolean;

    /**
     * Enable the onboarding checklist sidebar widget
     *
     * @default true
     */
    checklistWidget?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Filter args with a "target" on the type from the render function (EXPERIMENTAL)
     */
    argTypeTargetsV7?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Apply decorators from preview.js before decorators from addons or frameworks
     */
    legacyDecoratorFileOrder?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Disallow implicit actions during rendering. This will be the default in Storybook 8.
     *
     * This will make sure that your story renders the same no matter if docgen is enabled or not.
     */
    disallowImplicitActionsInRenderV8?: boolean;

    /**

Review comment (suggestion):
```suggestion
    onboardingInSidebar?: boolean;
```

I propose we link the featureFlag specifically to the sidebar, since that's what we're disabling.

I personally think the word "widget" has no meaning in storybook at the moment, so I'd prefer we avoided the word.

I agree with @Sidnioulz that `checklist` is a technical term, for the feature, and `onboarding` is more user-facing.

Code after:
     * Enable the integrated controls addon
     *
     * @default true
     */
    controls?: boolean;

    /**
     * Enable the integrated interactions addon
     *
     * @default true
     */
    interactions?: boolean;

    /**
     * Enable the integrated actions addon
     *
     * @default true
     */
    actions?: boolean;

    /**
     * Enable the onboarding checklist sidebar widget
     *
     * @default true
     */
    sidebarOnboardingChecklist?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Filter args with a "target" on the type from the render function (EXPERIMENTAL)
     */
    argTypeTargetsV7?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Apply decorators from preview.js before decorators from addons or frameworks
     */
    legacyDecoratorFileOrder?: boolean;

    /**
     * @temporary This feature flag is a migration assistant, and is scheduled to be removed.
     *
     * Disallow implicit actions during rendering. This will be the default in Storybook 8.
     *
     * This will make sure that your story renders the same no matter if docgen is enabled or not.
     */
    disallowImplicitActionsInRenderV8?: boolean;

    /**
Repository: storybookjs/storybook
File: code/core/src/manager/components/layout/useLandmarkIndicator.ts (TypeScript)

Code before:
import { useEffect, useRef } from 'react';

import { useTheme } from 'storybook/theming';

// Global keyboard handler for F6/Shift+F6 landmark navigation that
// highlights the landmark containing the current element. This helps
// users who navigate through landmark shortcuts more quickly visualise
// which region of the UI they landed into.
export function useLandmarkIndicator() {
  const theme = useTheme();
  const currentAnimationRef = useRef<Animation | null>(null);
  useEffect(() => {
    const handleKeyDown = (e: KeyboardEvent) => {
      if (e.key === 'F6') {
        let currentElement: Element | null = document.activeElement;
        let landmarkElement: HTMLElement | null = null;

        // Locate parent landmark by going up the DOM tree.
        while (currentElement) {
          if (
            currentElement instanceof HTMLElement &&
            currentElement.hasAttribute('data-sb-landmark')
          ) {
            landmarkElement = currentElement;
            break;
          }
          currentElement = currentElement.parentElement;
        }

        // Trigger the highlight animation on the found landmark.
        if (landmarkElement) {
          if (currentAnimationRef.current) {
            currentAnimationRef.current.cancel();
            currentAnimationRef.current = null;
          }

          const animation = landmarkElement.animate(
            [{ border: `2px solid ${theme.color.primary}` }, { border: `2px solid transparent` }],
            {

Review comment (suggestion):
Personal preference: 
```js
if (e.key !== 'F6') {
  return;
}
```
An early return reduces indentation for the rest of the function body, leaving more horizontal space for actual code.

Code after:
import { useEffect, useRef } from 'react';

import { useTheme } from 'storybook/theming';

function findActiveLandmarkElement() {
  let currentElement: Element | null = document.activeElement;
  let landmarkElement: HTMLElement | null = null;

  while (currentElement) {
    if (currentElement instanceof HTMLElement && currentElement.hasAttribute('data-sb-landmark')) {
      landmarkElement = currentElement;
      break;
    }
    currentElement = currentElement.parentElement;
  }

  return landmarkElement;
}

// Global keyboard handler for F6/Shift+F6 landmark navigation that
// highlights the landmark containing the current element. This helps
// users who navigate through landmark shortcuts more quickly visualise
// which region of the UI they landed into.
export function useLandmarkIndicator() {
  const theme = useTheme();
  const currentAnimationRef = useRef<Animation | null>(null);
  useEffect(() => {
    const handleKeyDown = (e: KeyboardEvent) => {
      if (e.key !== 'F6') {
        return;
      }

      const landmarkElement = findActiveLandmarkElement();
      if (!landmarkElement) {
        return;
      }

      // Cancel previous landmark animation if user switches fast.
      if (currentAnimationRef.current) {
Repository: storybookjs/storybook
File: code/core/src/manager/components/preview/Toolbar.tsx (TypeScript)

Code before:
      </Consumer>
    );
  },
};

export interface ToolData {
  isShown: boolean;
  tabs: Addon_BaseType[];
  tabState: TabListState<object>;
  tools: Addon_BaseType[];
  toolsExtra: Addon_BaseType[];
}

export const ToolbarComp = React.memo<ToolData>(function ToolbarComp({
  isShown,
  tools,
  toolsExtra,
  tabs,
  tabState,
}) {
  const sectionRef = useRef<HTMLElement>(null);
  const { landmarkProps } = useLandmark(
    {
      'aria-labelledby': 'sb-preview-toolbar-title',
      role: 'region',
    },
    sectionRef
  );

  return isShown && (tabs || tools || toolsExtra) ? (
    <StyledSection
      className="sb-bar"
      key="toolbar"
      data-testid="sb-preview-toolbar"
      ref={sectionRef}
      {...landmarkProps}
    >
      <h2 id="sb-preview-toolbar-title" className="sb-sr-only">
        Toolbar
      </h2>
      {tabs.length > 1 ? (
        <>
          <TabList state={tabState} />
          <Separator />
        </>
      ) : null}
      <StyledToolbar>
        <Tools key="left" list={tools} />
        <Tools key="right" list={toolsExtra} />
      </StyledToolbar>
    </StyledSection>

Review comment (suggestion):
```suggestion
    { 'aria-labelledby': 'sb-preview-toolbar-title', role: 'region' },
```
I would try to keep this compact, given the relative insignificance of this code. Same applies to other instances of `useLandmark`

Code after:
      </Consumer>
    );
  },
};

export interface ToolData {
  isShown: boolean;
  tabs: Addon_BaseType[];
  tabState: TabListState<object>;
  tools: Addon_BaseType[];
  toolsExtra: Addon_BaseType[];
}

export const ToolbarComp = React.memo<ToolData>(function ToolbarComp({
  isShown,
  tools,
  toolsExtra,
  tabs,
  tabState,
}) {
  const sectionRef = useRef<HTMLElement>(null);
  const { landmarkProps } = useLandmark(
    { 'aria-labelledby': 'sb-preview-toolbar-title', role: 'region' },
    sectionRef
  );

  return isShown && (tabs || tools || toolsExtra) ? (
    <StyledSection
      className="sb-bar"
      key="toolbar"
      data-testid="sb-preview-toolbar"
      ref={sectionRef}
      {...landmarkProps}
    >
      <h2 id="sb-preview-toolbar-title" className="sb-sr-only">
        Toolbar
      </h2>
      {tabs.length > 1 ? (
        <>
          <TabList state={tabState} />
          <Separator />
        </>
      ) : null}
      <StyledToolbar>
        <Tools key="left" list={tools} />
        <Tools key="right" list={toolsExtra} />
      </StyledToolbar>
    </StyledSection>
  ) : null;
});
Repository: storybookjs/storybook
File: code/e2e-tests/framework-vue3.spec.ts (TypeScript)

Code before:
test.describe('Vue 3', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto(storybookUrl);
    await new SbPage(page, expect).waitUntilLoaded();
  });

  test.skip(!templateName?.includes('vue3'), 'Only run these tests on Vue 3');

  test('updateArgs works in decorators', async ({ page }) => {
    const sbPage = new SbPage(page, expect);

    await sbPage.navigateToStory(
      'stories/renderers/vue3_vue3-vite-default-ts/decorators',
      'update-args'
    );
    const previewRoot = sbPage.previewRoot();
    const button = previewRoot.getByRole('button', { name: 'Add 1' });

    await expect(previewRoot).toContainText('0');
    await button.click();
    await expect(previewRoot).toContainText('1');
    await button.click();
    await expect(previewRoot).toContainText('2');
  });

  test('Decorators can inject reactive globals', async ({ page }) => {
    const sbPage = new SbPage(page, expect);

    await sbPage.navigateToStory(
      'stories/renderers/vue3_vue3-vite-default-ts/decorators',
      'reactive-global-decorator'
    );

    // Check the original language
    await expect(sbPage.previewRoot()).toContainText('Hello');

    // Select spanish in the locale toolbar and check that the text changes
    await sbPage.selectToolbar('[aria-label^="Internationalization locale"]', 'text=/Español/');
    await expect(sbPage.previewRoot()).toContainText('Hola');
  });
});


Review comment (suggestion):
```suggestion
  test('Decorators can consume reactive globals', async ({ page }) => {
```

Code after:
test.describe('Vue 3', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto(storybookUrl);
    await new SbPage(page, expect).waitUntilLoaded();
  });

  test.skip(!templateName?.includes('vue3'), 'Only run these tests on Vue 3');

  test('updateArgs works in decorators', async ({ page }) => {
    const sbPage = new SbPage(page, expect);

    await sbPage.navigateToStory(
      'stories/renderers/vue3_vue3-vite-default-ts/decorators',
      'update-args'
    );
    const previewRoot = sbPage.previewRoot();
    const button = previewRoot.getByRole('button', { name: 'Add 1' });

    await expect(previewRoot).toContainText('0');
    await button.click();
    await expect(previewRoot).toContainText('1');
    await button.click();
    await expect(previewRoot).toContainText('2');
  });

  test('Decorators can consume reactive globals', async ({ page }) => {
    const sbPage = new SbPage(page, expect);

    await sbPage.navigateToStory(
      'stories/renderers/vue3_vue3-vite-default-ts/decorators',
      'reactive-global-decorator'
    );

    // Check the original language
    await expect(sbPage.previewRoot()).toContainText('Hello');

    // Select spanish in the locale toolbar and check that the text changes
    await sbPage.selectToolbar('[aria-label^="Internationalization locale"]', 'text=/Español/');
    await expect(sbPage.previewRoot()).toContainText('Hola');
  });
});
Repository: storybookjs/storybook
File: code/core/src/components/components/Form/Input.tsx (TypeScript)

Code before:
  suffix?: string;
};

export const Input = Object.assign(
  styled(
    forwardRef<any, InputProps>(function Input(
      { size, valid, align, value, suffix, ...props },
      ref
    ) {
      return (
        <Wrapper>
          <input {...props} value={value} ref={ref} />
          {suffix && (
            <Mask aria-hidden>
              <span>{value}</span>
              <span>{suffix}</span>
            </Mask>
          )}
        </Wrapper>
      );
    })
  )<InputProps>(styles, sizes, alignment, validation, {
    minHeight: 32,
  }),
  {
    displayName: 'Input',
  }
);


Review comment (suggestion):
You could use `react-aris`'s useId to set an id on the `suffix` `span`, and use `aria-describedby={suffixId}` on the `input` to ensure it's announced by screen readers.

Code after:
  height?: number;
  suffix?: string;
};

export const Input = Object.assign(
  styled(
    forwardRef<any, InputProps>(function Input(
      { size, valid, align, value, suffix, ...props },
      ref
    ) {
      const suffixId = useId();
      return (
        <Wrapper>
          <input
            {...props}
            value={value}
            ref={ref}
            aria-describedby={suffix ? suffixId : undefined}
          />
          {suffix && (
            <Mask aria-hidden>
              <span>{value}</span>
              <span id={suffixId}>{suffix}</span>
            </Mask>
          )}
        </Wrapper>
      );
    })
  )<InputProps>(styles, sizes, alignment, validation, {
    minHeight: 32,
  }),
  {
    displayName: 'Input',
  }
);
Repository: storybookjs/storybook
File: code/addons/docs/src/blocks/controls/Object.tsx (TypeScript)

Code before:
  margin: placeholder ? 1 : '1px 0',
  padding: '3px 4px',
  color: theme.color.defaultText,
  background: theme.background.app,
  border: `1px solid ${theme.appBorderColor}`,
  borderRadius: 4,
  lineHeight: '14px',
  width: placeholder === 'Key' ? 80 : 120,
  '&:focus': {
    border: `1px solid ${theme.color.secondary}`,
  },
}));

const RawButton = styled(ToggleButton)({
  position: 'absolute',
  zIndex: 2,
  top: 2,
  right: 2,
  gap: '4px',

  // On small screens: remove absolute positioning, show only icon (WCAG 2.1 Reflow)
  '@media (max-width: 400px)': {
    position: 'static',
    alignSelf: 'flex-end',
    '& > span': {
      display: 'none',
    },
  },
});

const RawInput = styled(Form.Textarea)(({ theme }) => ({
  flex: 1,
  padding: '7px 6px',
  fontFamily: theme.typography.fonts.mono,
  fontSize: '12px',
  lineHeight: '18px',
  '&::placeholder': {
    fontFamily: theme.typography.fonts.base,
    fontSize: '13px',
  },
  '&:placeholder-shown': {
    padding: '7px 10px',
  },
}));

const ENTER_EVENT = {
  bubbles: true,
  cancelable: true,
  key: 'Enter',
  code: 'Enter',
  keyCode: 13,

Review comment (bug):
We can't entirely hide the text, or screen readers will fail to properly announce the edit button.

We have a `sb-sr-only` class available in `code/core/src/theming/global.ts`. Could you try to export the `CSSObject` defined for that class and then to inject it to your style here?

e.g.

```ts
import { srOnlyStyles } from '../../../ ... ' // or maybe from 'storybook/theming';

'@media (max-width: 400px)': {
  // ...
  '& > span': srOnlyStyles,
}
```

@ndelangen would you object to us exporting our screenreader-only class in `storybook/theming` to simplify this kind of use case? It's definitely a piece of code we'll keep very long term, and one which we're unlikely to ever need to adjust.

Code after:
  },
}));

const Input = styled.input(({ theme, placeholder }) => ({
  outline: 0,
  margin: placeholder ? 1 : '1px 0',
  padding: '3px 4px',
  color: theme.color.defaultText,
  background: theme.background.app,
  border: `1px solid ${theme.appBorderColor}`,
  borderRadius: 4,
  lineHeight: '14px',
  width: placeholder === 'Key' ? 80 : 120,
  '&:focus': {
    border: `1px solid ${theme.color.secondary}`,
  },
}));

const RawButton = styled(ToggleButton)({
  alignSelf: 'flex-start',
  order: 2,
  marginRight: -10,
});

const RawInput = styled(Form.Textarea)(({ theme }) => ({
  flex: 1,
  padding: '7px 6px',
  fontFamily: theme.typography.fonts.mono,
  fontSize: '12px',
  lineHeight: '18px',
  '&::placeholder': {
    fontFamily: theme.typography.fonts.base,
    fontSize: '13px',
  },
  '&:placeholder-shown': {
    padding: '7px 10px',
  },
}));

const ENTER_EVENT = {
  bubbles: true,
  cancelable: true,
  key: 'Enter',
  code: 'Enter',
  keyCode: 13,
};
const dispatchEnterKey = (event: SyntheticEvent<HTMLInputElement>) => {
  event.currentTarget.dispatchEvent(new globalWindow.KeyboardEvent('keydown', ENTER_EVENT));
};
const selectValue = (event: SyntheticEvent<HTMLInputElement>) => {
  event.currentTarget.select();