Skip to content

REFACTOR: Remove getType() override to allow extensibility#20

Merged
jsirish merged 1 commit intomasterfrom
fix/remove-gettype-override
Dec 2, 2025
Merged

REFACTOR: Remove getType() override to allow extensibility#20
jsirish merged 1 commit intomasterfrom
fix/remove-gettype-override

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 ElementEmbeddedCode
  • Updated $singular_name from 'Embedded Code Element' to 'Embedded Code'
  • Updated $plural_name from 'Embedded Code Elements' to 'Embedded Code Blocks'

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 #19

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.

Also updated $singular_name from 'Embedded Code Element' to
'Embedded Code' and $plural_name to 'Embedded Code Blocks' for
better consistency with other elements.

Fixes #19
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 refactors ElementEmbeddedCode to improve extensibility by removing the hardcoded getType() override and relying on the base class implementation that uses i18n_singular_name(). This allows developers to customize the element's display name through extensions by simply setting the $singular_name property rather than overriding the entire method.

Key Changes:

  • Removed the getType() method override to leverage BaseElement::getType()
  • Updated $singular_name to 'Embedded Code' (from 'Embedded Code Element') to maintain the same display name
  • Updated $plural_name to 'Embedded Code Blocks' (from 'Embedded Code Elements')

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

Comment on lines +23 to +28
private static $singular_name = 'Embedded Code';

/**
* @var string
*/
private static $plural_name = 'Embedded Code Elements';
private static $plural_name = 'Embedded Code Blocks';
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.

The plural name 'Embedded Code Blocks' is inconsistent with the singular name 'Embedded Code'. Typically, the plural should be the plural form of the singular. Consider changing $plural_name to 'Embedded Codes' to maintain consistency, or if you want to emphasize the block nature, consider changing $singular_name to 'Embedded Code Block' so that the plural follows naturally as 'Embedded Code Blocks'.

Copilot uses AI. Check for mistakes.
@jsirish
Copy link
Member Author

jsirish commented Dec 2, 2025

The plural naming convention 'X Blocks' is intentional and follows the pattern used across all SilverStripe Elemental modules. The singular name ('Embedded Code') is what users see in the element picker, while the plural name is used in admin contexts for listing multiple elements. Using "Blocks" as the plural suffix is consistent with the Elemental ecosystem.

@jsirish jsirish merged commit 16af7d7 into master Dec 2, 2025
24 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.

Update getType() to use $this->singular_name() for extensibility

2 participants