ronantakizawa/codereview-bench huggingface dataset:
Repository: ionic-team/ionic-framework
File: core/src/components/item/test/custom/item.e2e.ts (TypeScript)

Code before:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('item: custom'), () => {
    test.describe(title('CSS shadow parts'), () => {
      test('should be able to customize native part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-item::part(native) {
              background-color: red;
            }
          </style>

          <ion-item>
            <ion-label>Item</ion-label>
          </ion-item>
        `,
          config
        );

        const item = page.locator('ion-item');
        const backgroundColor = await item.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const native = shadowRoot?.querySelector('.item-native');
          return native ? window.getComputedStyle(native).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });


Review comment (suggestion):
```suggestion
    test.describe('CSS shadow parts', () => {
```

Nit: The `title` method just appends [mode/direction or mode/direction/palette](https://github.com/ionic-team/ionic-framework/blob/6f498a9da0ad0786856a692f6b0ccf6b2bbd728e/core/src/utils/test/playwright/generator.ts#L50) to the string. In this case, it's redundant to use it in nested describes like this, and usually when we have nested describes we don't use it twice.

Code after:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('item: custom'), () => {
    test.describe('CSS shadow parts', () => {
      test('should be able to customize native part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-item::part(native) {
              background-color: red;
            }
          </style>

          <ion-item>
            <ion-label>Item</ion-label>
          </ion-item>
        `,
          config
        );

        const item = page.locator('ion-item');
        const backgroundColor = await item.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const native = shadowRoot?.querySelector('.item-native');
          return native ? window.getComputedStyle(native).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });
Repository: ionic-team/ionic-framework
File: core/src/components/item-divider/test/custom/item-divider.e2e.ts (TypeScript)

Code before:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('item-divider: custom'), () => {
    test.describe(title('CSS shadow parts'), () => {
      test('should be able to customize inner part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-item-divider::part(inner) {
              background-color: red;
            }
          </style>

          <ion-item-divider>Divider</ion-item-divider>
        `,
          config
        );

        const divider = page.locator('ion-item-divider');
        const backgroundColor = await divider.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const inner = shadowRoot?.querySelector('.item-divider-inner');
          return inner ? window.getComputedStyle(inner).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });

      test('should be able to customize container part', async ({ page }) => {
        await page.setContent(

Review comment (suggestion):
```suggestion
    test.describe('CSS shadow parts', () => {
```

Nit: The `title` method just appends [mode/direction or mode/direction/palette](https://github.com/ionic-team/ionic-framework/blob/6f498a9da0ad0786856a692f6b0ccf6b2bbd728e/core/src/utils/test/playwright/generator.ts#L50) to the string. In this case, it's redundant to use it in nested describes like this, and usually when we have nested describes we don't use it twice.

Code after:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('item-divider: custom'), () => {
    test.describe('CSS shadow parts', () => {
      test('should be able to customize inner part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-item-divider::part(inner) {
              background-color: red;
            }
          </style>

          <ion-item-divider>Divider</ion-item-divider>
        `,
          config
        );

        const divider = page.locator('ion-item-divider');
        const backgroundColor = await divider.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const inner = shadowRoot?.querySelector('.item-divider-inner');
          return inner ? window.getComputedStyle(inner).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });

      test('should be able to customize container part', async ({ page }) => {
        await page.setContent(
Repository: ionic-team/ionic-framework
File: core/src/components/item-option/test/custom/item-option.e2e.ts (TypeScript)

Code before:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('item-option: custom'), () => {
    test.describe(title('CSS shadow parts'), () => {
      test('should be able to customize native part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-item-option::part(native) {
              background-color: red;
            }
          </style>

          <ion-item-option>Option</ion-item-option>
        `,
          config
        );

        const itemOption = page.locator('ion-item-option');
        const backgroundColor = await itemOption.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const native = shadowRoot?.querySelector('.button-native');
          return native ? window.getComputedStyle(native).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });

      test('should be able to customize inner part', async ({ page }) => {
        await page.setContent(

Review comment (suggestion):
```suggestion
    test.describe('CSS shadow parts', () => {
```

Nit: The `title` method just appends [mode/direction or mode/direction/palette](https://github.com/ionic-team/ionic-framework/blob/6f498a9da0ad0786856a692f6b0ccf6b2bbd728e/core/src/utils/test/playwright/generator.ts#L50) to the string. In this case, it's redundant to use it in nested describes like this, and usually when we have nested describes we don't use it twice.

Code after:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('item-option: custom'), () => {
    test.describe('CSS shadow parts', () => {
      test('should be able to customize native part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-item-option::part(native) {
              background-color: red;
            }
          </style>

          <ion-item-option>Option</ion-item-option>
        `,
          config
        );

        const itemOption = page.locator('ion-item-option');
        const backgroundColor = await itemOption.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const native = shadowRoot?.querySelector('.button-native');
          return native ? window.getComputedStyle(native).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });

      test('should be able to customize inner part', async ({ page }) => {
        await page.setContent(
Repository: ionic-team/ionic-framework
File: core/src/components/list-header/test/custom/list-header.e2e.ts (TypeScript)

Code before:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('list-header: custom'), () => {
    test.describe(title('CSS shadow parts'), () => {
      test('should be able to customize inner part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-list-header::part(inner) {
              background-color: red;
            }
          </style>

          <ion-list-header>Header</ion-list-header>
        `,
          config
        );

        const header = page.locator('ion-list-header');
        const backgroundColor = await header.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const inner = shadowRoot?.querySelector('.list-header-inner');
          return inner ? window.getComputedStyle(inner).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });
    });
  });
});

Review comment (suggestion):
```suggestion
    test.describe('CSS shadow parts', () => {
```

Nit: The `title` method just appends [mode/direction or mode/direction/palette](https://github.com/ionic-team/ionic-framework/blob/6f498a9da0ad0786856a692f6b0ccf6b2bbd728e/core/src/utils/test/playwright/generator.ts#L50) to the string. In this case, it's redundant to use it in nested describes like this, and usually when we have nested describes we don't use it twice.

Code after:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across modes/directions
 */
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
  test.describe(title('list-header: custom'), () => {
    test.describe('CSS shadow parts', () => {
      test('should be able to customize inner part', async ({ page }) => {
        await page.setContent(
          `
          <style>
            ion-list-header::part(inner) {
              background-color: red;
            }
          </style>

          <ion-list-header>Header</ion-list-header>
        `,
          config
        );

        const header = page.locator('ion-list-header');
        const backgroundColor = await header.evaluate((el) => {
          const shadowRoot = el.shadowRoot;
          const inner = shadowRoot?.querySelector('.list-header-inner');
          return inner ? window.getComputedStyle(inner).backgroundColor : '';
        });
        expect(backgroundColor).toBe('rgb(255, 0, 0)');
      });
    });
  });
});
Repository: ionic-team/ionic-framework
File: core/src/components/list-header/list-header.tsx (TypeScript)

Code before:
import type { ComponentInterface } from '@stencil/core';
import { Component, Host, Prop, h } from '@stencil/core';
import { createColorClasses } from '@utils/theme';

import { getIonMode } from '../../global/ionic-global';
import type { Color } from '../../interface';

/**
 * @virtualProp {"ios" | "md"} mode - The mode determines which platform styles to use.
 *
 * @part inner - The inner container element that wraps the list header content.
 */
@Component({
  tag: 'ion-list-header',
  styleUrls: {
    ios: 'list-header.ios.scss',
    md: 'list-header.md.scss',
  },
  shadow: true,
})
export class ListHeader implements ComponentInterface {
  /**
   * The color to use from your application's color palette.
   * Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`.
   * For more information on colors, see [theming](/docs/theming/basics).
   */
  @Prop({ reflect: true }) color?: Color;

  /**
   * How the bottom border should be displayed on the list header.
   */
  @Prop() lines?: 'full' | 'inset' | 'none';

  render() {
    const { lines } = this;
    const mode = getIonMode(this);

Review comment (suggestion):
```suggestion
 * @part inner - The inner wrapper element that arranges the list header content.
```
Nit: Avoid confusing terminology since container is a part

Code after:
import type { ComponentInterface } from '@stencil/core';
import { Component, Host, Prop, h } from '@stencil/core';
import { createColorClasses } from '@utils/theme';

import { getIonMode } from '../../global/ionic-global';
import type { Color } from '../../interface';

/**
 * @virtualProp {"ios" | "md"} mode - The mode determines which platform styles to use.
 *
 * @part inner - The inner wrapper element that arranges the list header content.
 */
@Component({
  tag: 'ion-list-header',
  styleUrls: {
    ios: 'list-header.ios.scss',
    md: 'list-header.md.scss',
  },
  shadow: true,
})
export class ListHeader implements ComponentInterface {
  /**
   * The color to use from your application's color palette.
   * Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`.
   * For more information on colors, see [theming](/docs/theming/basics).
   */
  @Prop({ reflect: true }) color?: Color;

  /**
   * How the bottom border should be displayed on the list header.
   */
  @Prop() lines?: 'full' | 'inset' | 'none';

  render() {
    const { lines } = this;
    const mode = getIonMode(this);
Repository: ionic-team/ionic-framework
File: core/src/components/refresher/test/basic/refresher.e2e.ts (TypeScript)

Code before:
        const refresherContent = page.locator('ion-refresher-content');
        refresherContent.evaluateHandle((el: any) => {
          // Resets the pullingIcon to enable the native refresher
          el.pullingIcon = undefined;
        });

        await page.waitForChanges();

        const items = page.locator('ion-item');
        expect(await items.count()).toBe(30);

        await pullToRefresh(page);

        expect(await items.count()).toBe(60);
      });

      test('should emit ionPullStart and ionPullEnd with reason complete', async ({ page }) => {
        const refresherContent = page.locator('ion-refresher-content');
        refresherContent.evaluateHandle((el: any) => {
          // Resets the pullingIcon to enable the native refresher
          el.pullingIcon = undefined;
        });

        await page.waitForChanges();

        await page.locator('ion-refresher.hydrated').waitFor({ state: 'attached' });

        const ionPullStartEvent = await page.spyOnEvent('ionPullStartFired');
        const ionPullEndEvent = await page.spyOnEvent('ionPullEndFired');

        await pullToRefresh(page);

        // Wait for the reset animation to complete (native refresher takes longer due to CSS transitions)
        await page.waitForTimeout(1500);

        expect(ionPullStartEvent).toHaveReceivedEventTimes(1);
        expect(ionPullEndEvent).toHaveReceivedEventTimes(1);
        expect(ionPullEndEvent).toHaveReceivedEventDetail({ reason: 'complete' });
      });
    });
  });
});


Review comment (suggestion):
Same [question](https://github.com/ionic-team/ionic-framework/pull/30946/changes#r2783854946) applies here.

Code after:
        });

        await page.waitForChanges();

        const items = page.locator('ion-item');
        expect(await items.count()).toBe(30);

        await pullToRefresh(page);

        expect(await items.count()).toBe(60);
      });

      test('should emit ionPullStart and ionPullEnd with reason complete', async ({ page }) => {
        const refresherContent = page.locator('ion-refresher-content');
        refresherContent.evaluateHandle((el: any) => {
          // Resets the pullingIcon to enable the native refresher
          el.pullingIcon = undefined;
        });

        await page.waitForChanges();

        const ionPullStartEvent = await page.spyOnEvent('ionPullStartFired');
        const ionPullEndEvent = await page.spyOnEvent('ionPullEndFired');

        await pullToRefresh(page);

        // Wait for the reset animation to complete (native refresher takes longer due to CSS transitions)
        await page.waitForTimeout(1500);

        expect(ionPullStartEvent).toHaveReceivedEventTimes(1);
        expect(ionPullEndEvent).toHaveReceivedEventTimes(1);
        expect(ionPullEndEvent).toHaveReceivedEventDetail({ reason: 'complete' });
      });
    });
  });
});
Repository: ionic-team/ionic-framework
File: core/src/components/textarea/test/rows/textarea.e2e.ts (TypeScript)

Code before:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * Rows attribute is only available in the Ionic theme.
 * When set, it increases the container min-height to accommodate the number of rows.
 */
configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
  test.describe(title('textarea: rows'), () => {
    test('should respect rows attribute and set min-height', async ({ page }) => {
      await page.setContent(
        `
        <ion-textarea
          rows="3"
          label="Comments"
          label-placement="stacked"
          helper-text="rows=3, label-placement=stacked"
          value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
        ></ion-textarea>
      `,
        config
      );

      const textarea = page.locator('ion-textarea');
      await expect(textarea).toHaveScreenshot(screenshot(`textarea-rows-3`));
    });

    test('should respect rows attribute with different values', async ({ page }) => {
      await page.setContent(
        `
        <div id="container" style="display: flex; flex-direction: column; gap: 20px;">

Review comment (suggestion):
Let's be consistent with the other tests:


```suggestion
 * This behavior does not vary across directions
 * This behavior only applies to the `ionic` theme.
```

Code after:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across directions
 * This behavior only applies to the `ionic` theme.
 */
configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
  test.describe(title('textarea: rows'), () => {
    test('should respect rows attribute', async ({ page }) => {
      await page.setContent(
        `
        <div id="container" style="display: flex; flex-direction: column; gap: 20px;">
          <ion-textarea
            rows="1"
            label="Comments"
            value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
          ></ion-textarea>
          <ion-textarea
            rows="3"
            label="Comments"
            value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
          ></ion-textarea>
          <ion-textarea
            rows="8"
            label="Comments"
            value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
          ></ion-textarea>
        </div>
      `,
        config
Repository: ionic-team/ionic-framework
File: core/src/components/textarea/test/rows/textarea.e2e.ts (TypeScript)

Code before:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * Rows attribute is only available in the Ionic theme.
 * When set, it increases the container min-height to accommodate the number of rows.
 */
configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
  test.describe(title('textarea: rows'), () => {
    test('should respect rows attribute and set min-height', async ({ page }) => {
      await page.setContent(
        `
        <ion-textarea
          rows="3"
          label="Comments"
          label-placement="stacked"
          helper-text="rows=3, label-placement=stacked"
          value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
        ></ion-textarea>
      `,
        config
      );

      const textarea = page.locator('ion-textarea');
      await expect(textarea).toHaveScreenshot(screenshot(`textarea-rows-3`));
    });

    test('should respect rows attribute with different values', async ({ page }) => {
      await page.setContent(
        `
        <div id="container" style="display: flex; flex-direction: column; gap: 20px;">
          <ion-textarea
            rows="1"
            label="Small"
            label-placement="stacked"

Review comment (suggestion):
```suggestion
    test('should respect rows attribute, async ({ page }) => {
```

Code after:
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

/**
 * This behavior does not vary across directions
 * This behavior only applies to the `ionic` theme.
 */
configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
  test.describe(title('textarea: rows'), () => {
    test('should respect rows attribute', async ({ page }) => {
      await page.setContent(
        `
        <div id="container" style="display: flex; flex-direction: column; gap: 20px;">
          <ion-textarea
            rows="1"
            label="Comments"
            value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
          ></ion-textarea>
          <ion-textarea
            rows="3"
            label="Comments"
            value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
          ></ion-textarea>
          <ion-textarea
            rows="8"
            label="Comments"
            value="1&#10;2&#10;3&#10;4&#10;5&#10;6&#10;7&#10;8&#10;9&#10;0"
          ></ion-textarea>
        </div>
      `,
        config
      );

      const container = page.locator('#container');
      await expect(container).toHaveScreenshot(screenshot(`textarea-rows`));