Skip to content

Conversation

@jsirish
Copy link
Member

@jsirish jsirish commented Dec 2, 2025

Summary

Remove the getType() method override so the element inherits BaseElement::getType() which uses i18n_singular_name(). This allows sites to customize the element's display name via extensions by setting $singular_name.

Changes

  • Removed the getType() method from ElementCarousel

Rationale

The base BaseElement::getType() implementation already uses $this->i18n_singular_name():

public function getType()
{
    $default = $this->i18n_singular_name() ?: 'Block';
    return _t(static::class . '.BlockType', $default);
}

By removing the override, extensions can now customize the display name in the CMS element picker by simply setting $singular_name, without needing to override the entire method.

Fixes #13

Related: dynamic/silverstripe-essentials-tools#68

Remove the getType() method override so the element inherits
BaseElement::getType() which uses i18n_singular_name(). This allows
sites to customize the element's display name via extensions by
setting $singular_name.

Fixes #13
Copilot AI review requested due to automatic review settings December 2, 2025 06:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the getType() method override from ElementCarousel to enable extensibility through the base class's i18n_singular_name() pattern, allowing sites to customize the element's display name via configuration rather than method overrides.

Key Changes

  • Removed the getType() method that previously returned a hardcoded translation of 'Carousel'
  • Element will now inherit BaseElement::getType() which uses i18n_singular_name() for the default value

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{
return _t(__CLASS__ . '.BlockType', 'Carousel');
}
}
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Removing getType() without adding private static $singular_name = 'Carousel'; to the class will break backward compatibility. Without $singular_name, BaseElement::getType() will fall back to 'Block' as the default display name instead of 'Carousel'.

Additionally, the test testGetType() in tests/Elements/ElementCarouselTest.php (line 38-42) expects getType() to return 'Carousel' and will fail after this change unless updated.

Copilot uses AI. Check for mistakes.
Address Copilot review feedback - ensure getType() returns correct value
after removing the method override.
@jsirish jsirish merged commit 70aad6f into 2 Dec 2, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants