ronantakizawa/codereview-bench huggingface dataset:
Repository: ionic-team/ionic-framework
File: core/src/components/range/range.tsx (TypeScript)

Code before:
          this.el
        );
        this.activeBarStart = this.min;
      }
    }
  }

  /**
   * If `true`, the user cannot interact with the range.
   */
  @Prop() disabled = false;
  @Watch('disabled')
  protected disabledChanged() {
    if (this.gesture) {
      this.gesture.enable(!this.disabled);
    }
  }

  /**
   * the value of the range.
   */
  @Prop({ mutable: true }) value: RangeValue = 0;
  @Watch('value')
  protected valueChanged(newVlue: RangeValue, oldValue: RangeValue) {
    // The check is necessary because the value can be an object.
    if (JSON.stringify(newVlue) !== JSON.stringify(oldValue)) {
      this.ionInput.emit({ value: this.value });
    }

    if (!this.noUpdate) {
      this.updateRatio();
    }
  }

  private clampBounds = (value: any): number => {
    return clamp(this.min, value, this.max);
  };

  private ensureValueInBounds = (value: any) => {
    if (this.dualKnobs) {
      return {
        lower: this.clampBounds(value.lower),
        upper: this.clampBounds(value.upper),
      };
    } else {
      return this.clampBounds(value);
    }
  };

  /**
   * Where to place the label relative to the range.

Review comment (performance):
JSON.stringify is a bit slow. I don't know that it super matters in the long run, but it would be significantly more efficient if you compared the properties without stringifying them. I know it's a bit annoying because they're a union type of a number and object, but worth considering I guess.

Code after:
          this.el
        );
        this.activeBarStart = this.min;
      }
    }
  }

  /**
   * If `true`, the user cannot interact with the range.
   */
  @Prop() disabled = false;
  @Watch('disabled')
  protected disabledChanged() {
    if (this.gesture) {
      this.gesture.enable(!this.disabled);
    }
  }

  /**
   * the value of the range.
   */
  @Prop({ mutable: true }) value: RangeValue = 0;
  @Watch('value')
  protected valueChanged(newValue: RangeValue, oldValue: RangeValue) {
    const valuesChanged = this.compareValues(newValue, oldValue);
    if (valuesChanged) {
      this.ionInput.emit({ value: this.value });
    }

    if (!this.noUpdate) {
      this.updateRatio();
    }
  }

  /**
   * Compares two RangeValue inputs to determine if they are different.
   *
   * @param newVal - The new value.
   * @param oldVal - The old value.
   * @returns `true` if the values are different, `false` otherwise.
   */
  private compareValues = (newVal: RangeValue, oldVal: RangeValue) => {
    if (typeof newVal === 'object' && typeof oldVal === 'object') {
      return newVal.lower !== oldVal.lower || newVal.upper !== oldVal.upper;
    }

    return newVal !== oldVal;
  };

  private clampBounds = (value: any): number => {
    return clamp(this.min, value, this.max);
Repository: ionic-team/ionic-framework
File: core/src/components/checkbox/checkbox.tsx (TypeScript)

Code before:
  private setChecked = (state: boolean) => {
    const isChecked = (this.checked = state);
    this.ionChange.emit({
      checked: isChecked,
      value: this.value,
    });
  };

  private toggleChecked = (ev: Event) => {
    ev.preventDefault();

    this.setFocus();
    this.setChecked(!this.checked);
    this.indeterminate = false;
  };

  private onFocus = () => {
    this.ionFocus.emit();
  };

  private onBlur = () => {
    this.ionBlur.emit();
  };

  private onKeyDown = (ev: KeyboardEvent) => {
    if (ev.key === ' ' || ev.key === 'Enter') {
      ev.preventDefault();
      if (!this.disabled) {
        this.toggleChecked(ev);
      }
    }
  };

  private onClick = (ev: MouseEvent) => {
    if (this.disabled) {
      return;
    }

    this.toggleChecked(ev);
  };

  private getHintTextID(): string | undefined {
    const { el, helperText, errorText, helperTextId, errorTextId } = this;

    if (el.classList.contains('ion-touched') && el.classList.contains('ion-invalid') && errorText) {
      return errorTextId;
    }

    if (helperText) {
      return helperTextId;
    }

Review comment (suggestion):
Based on [W3C](https://www.w3.org/WAI/ARIA/apg/patterns/checkbox/examples/checkbox/#kbd_label), only the `SPACE` key should toggle. I also verified with Chakra UI, a library known for it's accessibility.

```suggestion
    if (ev.key === ' ') {
```

Code after:
   */
  private setChecked = (state: boolean) => {
    const isChecked = (this.checked = state);
    this.ionChange.emit({
      checked: isChecked,
      value: this.value,
    });
  };

  private toggleChecked = (ev: Event) => {
    ev.preventDefault();

    this.setFocus();
    this.setChecked(!this.checked);
    this.indeterminate = false;
  };

  private onFocus = () => {
    this.ionFocus.emit();
  };

  private onBlur = () => {
    this.ionBlur.emit();
  };

  private onKeyDown = (ev: KeyboardEvent) => {
    if (ev.key === ' ') {
      ev.preventDefault();
      if (!this.disabled) {
        this.toggleChecked(ev);
      }
    }
  };

  private onClick = (ev: MouseEvent) => {
    if (this.disabled) {
      return;
    }

    this.toggleChecked(ev);
  };

  private getHintTextID(): string | undefined {
    const { el, helperText, errorText, helperTextId, errorTextId } = this;

    if (el.classList.contains('ion-touched') && el.classList.contains('ion-invalid') && errorText) {
      return errorTextId;
    }

    if (helperText) {
      return helperTextId;
Repository: ionic-team/ionic-framework
File: core/src/components/badge/badge.tsx (TypeScript)

Code before:
    if (shape === undefined) {
      return 'round';
    }

    return shape;
  }

  private getSize(): string | undefined {
    const theme = getIonTheme(this);
    const { size } = this;

    // TODO(ROU-10747): Remove theme check when sizes are defined for all themes.
    if (theme !== 'ionic') {
      return undefined;
    }

    if (size === undefined) {
      return 'small';
    }

    return size;
  }

  // The 'subtle' hue is the default for badges containing text or icons
  // The 'bold' hue is used when inside of an avatar, button, tab button,
  // or when the badge is empty (no text or icon)
  private getHue(): string | undefined {
    const { hue } = this;

    if (hue !== undefined) {
      return hue;
    }

    const inAvatar = hostContext('ion-avatar', this.el);
    const inButton = hostContext('ion-button', this.el);
    const inTabButton = hostContext('ion-tab-button', this.el);
    const hasContent = this.el.textContent?.trim() !== '' || this.el.querySelector('ion-icon') !== null;

    // Return 'bold' if the badge is inside an avatar, button, tab button,
    // or has no content
    if (inAvatar || inButton || inTabButton || !hasContent) {
      return 'bold';
    }

    // Return 'subtle' if the badge contains visible text or an icon
    return 'subtle';
  }

  render() {
    const hue = this.getHue();
    const shape = this.getShape();

Review comment (suggestion):
```suggestion
  // The 'subtle' hue is the default for badges containing text or icons.
  // The 'bold' hue is used when inside of an avatar, button, tab button,
  // or when the badge is empty (no text or icon).
```

Code after:
    }

    if (shape === undefined) {
      return 'round';
    }

    return shape;
  }

  private getSize(): string | undefined {
    const theme = getIonTheme(this);
    const { size } = this;

    // TODO(ROU-10747): Remove theme check when sizes are defined for all themes.
    if (theme !== 'ionic') {
      return undefined;
    }

    if (size === undefined) {
      return 'small';
    }

    return size;
  }

  // The 'subtle' hue is the default for badges containing text or icons
  // The 'bold' hue is used when inside of an avatar, button, tab button,
  // or when the badge is empty (no text or icon).
  private getHue(): string | undefined {
    const { hue } = this;

    if (hue !== undefined) {
      return hue;
    }

    const inAvatar = hostContext('ion-avatar', this.el);
    const inButton = hostContext('ion-button', this.el);
    const inTabButton = hostContext('ion-tab-button', this.el);
    const hasContent = this.el.textContent?.trim() !== '' || this.el.querySelector('ion-icon') !== null;

    // Return 'bold' if the badge is inside an avatar, button, tab button,
    // or has no content
    if (inAvatar || inButton || inTabButton || !hasContent) {
      return 'bold';
    }

    // Return 'subtle' if the badge contains visible text or an icon
    return 'subtle';
  }

  render() {
Repository: ionic-team/ionic-framework
File: core/src/components/textarea/test/basic/textarea.e2e.ts (TypeScript)

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

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
  test.describe(title('textarea: click'), () => {
    test('should trigger onclick only once when clicking the label', async ({ page }) => {
      // Create a spy function in page context
      await page.setContent(`<ion-textarea label="Textarea"></ion-textarea>`, config);

      // Track calls to the exposed function
      const clickEvent = await page.spyOnEvent('click');
      const input = page.locator('label.textarea-wrapper');

      // Use position to make sure we click into the label enough to trigger
      // what would be the double click
      await input.click({
        position: {
          x: 5,
          y: 5,
        },
      });

      // Verify the click was triggered exactly once
      expect(clickEvent).toHaveReceivedEventTimes(1);

      // Verify that the event target is the checkbox and not the item
      const event = clickEvent.events[0];
      expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-textarea');
    });
  });
});


Review comment (suggestion):
```suggestion
      testInfo.annotations.push({
        type: 'issue',
        description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
      });
      // Create a spy function in page context
```

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

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
  test.describe(title('textarea: click'), () => {
    test('should trigger onclick only once when clicking the label', async ({ page }, testInfo) => {
      testInfo.annotations.push({
        type: 'issue',
        description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
      });
      // Create a spy function in page context
      await page.setContent(`<ion-textarea label="Textarea"></ion-textarea>`, config);

      // Track calls to the exposed function
      const clickEvent = await page.spyOnEvent('click');
      const input = page.locator('label.textarea-wrapper');

      // Use position to make sure we click into the label enough to trigger
      // what would be the double click
      await input.click({
        position: {
          x: 5,
          y: 5,
        },
      });

      // Verify the click was triggered exactly once
      expect(clickEvent).toHaveReceivedEventTimes(1);

      // Verify that the event target is the checkbox and not the item
      const event = clickEvent.events[0];
      expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-textarea');
    });
  });
});
Repository: ionic-team/ionic-framework
File: core/src/components/toggle/test/basic/toggle.e2e.ts (TypeScript)

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

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
  test.describe(title('toggle: click'), () => {
    test('should trigger onclick only once when clicking the label', async ({ page }) => {
      // Create a spy function in page context
      await page.setContent(`<ion-toggle onclick="console.log('click called')">my label</ion-toggle>`, config);

      // Track calls to the exposed function
      let clickCount = 0;
      page.on('console', (msg) => {
        if (msg.text().includes('click called')) {
          clickCount++;
        }
      });

      const input = page.locator('div.label-text-wrapper');

      // Use position to make sure we click into the label enough to trigger
      // what would be the double click
      await input.click({
        position: {
          x: 5,
          y: 5,
        },
      });

      // Verify the click was triggered exactly once
      expect(clickCount).toBe(1);
    });
  });
});


Review comment (suggestion):
```suggestion
      testInfo.annotations.push({
        type: 'issue',
        description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
      });
      // Create a spy function in page context
```

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

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
  test.describe(title('toggle: click'), () => {
    test('should trigger onclick only once when clicking the label', async ({ page }, testInfo) => {
      testInfo.annotations.push({
        type: 'issue',
        description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
      });

      // Create a spy function in page context
      await page.setContent(`<ion-toggle onclick="console.log('click called')">my label</ion-toggle>`, config);

      // Track calls to the exposed function
      let clickCount = 0;
      page.on('console', (msg) => {
        if (msg.text().includes('click called')) {
          clickCount++;
        }
      });

      const input = page.locator('div.label-text-wrapper');

      // Use position to make sure we click into the label enough to trigger
      // what would be the double click
      await input.click({
        position: {
          x: 5,
          y: 5,
        },
      });

      // Verify the click was triggered exactly once
      expect(clickCount).toBe(1);
    });
  });
Repository: ionic-team/ionic-framework
File: core/src/components/button/button.tsx (TypeScript)

Code before:

    // If the button is in a toolbar, has a clear fill (which is the default)
    // and only has an icon we use the unbounded "circular" ripple effect
    if (hasClearFill && this.hasIconOnly && this.inToolbar) {
      return 'unbounded';
    }

    return 'bounded';
  }

  /**
   * Set the shape based on the theme
   */
  private getShape(): string {
    const theme = getIonTheme(this);
    const { shape } = this;

    if (shape === undefined) {
      return theme === 'ios' ? 'soft' : 'round';
    }

    return shape;
  }

  /**
   * Set the size based if the button is inside of an item, the default size is "small"
   */
  private getSize(): string | undefined {
    const { size } = this;

    if (size === undefined && this.inItem) {
      return 'small';
    }

    return size;
  }

  /**
   * Finds the form element based on the provided `form` selector
   * or element reference provided.
   */
  private findForm(): HTMLFormElement | null {
    const { form } = this;
    if (form instanceof HTMLFormElement) {
      return form;
    }
    if (typeof form === 'string') {
      // Check if the string provided is a form id.
      const el: HTMLElement | null = document.getElementById(form);
      if (el) {
        if (el instanceof HTMLFormElement) {

Review comment (suggestion):
```suggestion
   * Get the default size. If size is unset and the button is inside an item, 
   * the default size is "small". Otherwise, it uses the value of size.
```

Code after:

    // If the button is in a toolbar, has a clear fill (which is the default)
    // and only has an icon we use the unbounded "circular" ripple effect
    if (hasClearFill && this.hasIconOnly && this.inToolbar) {
      return 'unbounded';
    }

    return 'bounded';
  }

  /**
   * Set the shape based on the theme
   */
  private getShape(): string {
    const theme = getIonTheme(this);
    const { shape } = this;

    if (shape === undefined) {
      return theme === 'ios' ? 'soft' : 'round';
    }

    return shape;
  }

  /**
   * Get the default size. If size is unset and the button is inside an item,
   * the default size is "small". Otherwise, it uses the value of size.
   */
  private getSize(): string | undefined {
    const { size } = this;

    if (size === undefined) {
      return this.inItem ? 'small' : 'medium';
    }

    return size;
  }

  /**
   * Finds the form element based on the provided `form` selector
   * or element reference provided.
   */
  private findForm(): HTMLFormElement | null {
    const { form } = this;
    if (form instanceof HTMLFormElement) {
      return form;
    }
    if (typeof form === 'string') {
      // Check if the string provided is a form id.
      const el: HTMLElement | null = document.getElementById(form);
      if (el) {
Repository: ionic-team/ionic-framework
File: core/src/components/button/button.tsx (TypeScript)

Code before:
    return 'bounded';
  }

  /**
   * Set the shape based on the theme
   */
  private getShape(): string {
    const theme = getIonTheme(this);
    const { shape } = this;

    if (shape === undefined) {
      return theme === 'ios' ? 'soft' : 'round';
    }

    return shape;
  }

  /**
   * Set the size based if the button is inside of an item, the default size is "small"
   */
  private getSize(): string | undefined {
    const { size } = this;

    if (size === undefined && this.inItem) {
      return 'small';
    }

    return size;
  }

  /**
   * Finds the form element based on the provided `form` selector
   * or element reference provided.
   */
  private findForm(): HTMLFormElement | null {
    const { form } = this;
    if (form instanceof HTMLFormElement) {
      return form;
    }
    if (typeof form === 'string') {
      // Check if the string provided is a form id.
      const el: HTMLElement | null = document.getElementById(form);
      if (el) {
        if (el instanceof HTMLFormElement) {
          return el;
        } else {
          /**
           * The developer specified a string for the form attribute, but the
           * element with that id is not a form element.
           */
          printIonWarning(

Review comment (suggestion):
```suggestion
    if (size === undefined) {
      return this.inItem ? 'small' : 'medium';
    }
```

If we go with adding `medium` above

Code after:
    return 'bounded';
  }

  /**
   * Set the shape based on the theme
   */
  private getShape(): string {
    const theme = getIonTheme(this);
    const { shape } = this;

    if (shape === undefined) {
      return theme === 'ios' ? 'soft' : 'round';
    }

    return shape;
  }

  /**
   * Get the default size. If size is unset and the button is inside an item,
   * the default size is "small". Otherwise, it uses the value of size.
   */
  private getSize(): string | undefined {
    const { size } = this;

    if (size === undefined) {
      return this.inItem ? 'small' : 'medium';
    }

    return size;
  }

  /**
   * Finds the form element based on the provided `form` selector
   * or element reference provided.
   */
  private findForm(): HTMLFormElement | null {
    const { form } = this;
    if (form instanceof HTMLFormElement) {
      return form;
    }
    if (typeof form === 'string') {
      // Check if the string provided is a form id.
      const el: HTMLElement | null = document.getElementById(form);
      if (el) {
        if (el instanceof HTMLFormElement) {
          return el;
        } else {
          /**
           * The developer specified a string for the form attribute, but the
           * element with that id is not a form element.
           */
Repository: ionic-team/ionic-framework
File: core/src/components/router/router.tsx (TypeScript)

Code before:
      this.back();
      processNextHandler();
    });
  }

  /** @internal */
  @Method()
  async canTransition() {
    const canProceed = await this.runGuards();
    if (canProceed !== true) {
      if (typeof canProceed === 'object') {
        return canProceed.redirect;
      } else {
        return false;
      }
    }

    return true;
  }

  /**
   * Navigate to the specified path.
   *
   * @param path The path to navigate to.
   * @param direction The direction of the animation. Defaults to `"forward"`.
   * @param animation The animation to play when navigating.
   */
  @Method()
  async push(path: string, direction: RouterDirection = 'forward', animation?: AnimationBuilder) {
    if (path.startsWith('.')) {
      const currentPath = this.previousPath ?? '/';
      // Convert currentPath to an URL by pre-pending a protocol and a host to resolve the relative path.
      const url = new URL(path, `https://host/${currentPath}`);
      path = url.pathname + url.search;
    }

    let parsedPath = parsePath(path);

    const canProceed = await this.runGuards(parsedPath.segments);
    if (canProceed !== true) {
      if (typeof canProceed === 'object') {
        parsedPath = parsePath(canProceed.redirect);
      } else {
        return false;
      }
    }

    this.setSegments(parsedPath.segments, direction, parsedPath.queryString);
    return this.writeNavStateRoot(parsedPath.segments, direction, animation);
  }


Review comment (suggestion):
```suggestion
   * @param animation A custom animation to use for the transition.
```

Code after:
      this.back();
      processNextHandler();
    });
  }

  /** @internal */
  @Method()
  async canTransition() {
    const canProceed = await this.runGuards();
    if (canProceed !== true) {
      if (typeof canProceed === 'object') {
        return canProceed.redirect;
      } else {
        return false;
      }
    }

    return true;
  }

  /**
   * Navigate to the specified path.
   *
   * @param path The path to navigate to.
   * @param direction The direction of the animation. Defaults to `"forward"`.
   * @param animation A custom animation to use for the transition.
   */
  @Method()
  async push(path: string, direction: RouterDirection = 'forward', animation?: AnimationBuilder) {
    if (path.startsWith('.')) {
      const currentPath = this.previousPath ?? '/';
      // Convert currentPath to an URL by pre-pending a protocol and a host to resolve the relative path.
      const url = new URL(path, `https://host/${currentPath}`);
      path = url.pathname + url.search;
    }

    let parsedPath = parsePath(path);

    const canProceed = await this.runGuards(parsedPath.segments);
    if (canProceed !== true) {
      if (typeof canProceed === 'object') {
        parsedPath = parsePath(canProceed.redirect);
      } else {
        return false;
      }
    }

    this.setSegments(parsedPath.segments, direction, parsedPath.queryString);
    return this.writeNavStateRoot(parsedPath.segments, direction, animation);
  }