Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Move functions into FieldDropdown. #8634

Merged
merged 3 commits into from
Nov 8, 2024

Conversation

gonfunko
Copy link
Contributor

The basics

The details

Resolves

Fixes #6349

Proposed Changes

This PR moves several functions that were in field_dropdown.ts, not exported, and used only by FieldDropdown, to simply be members of the FieldDropdown class in order to allow subclasses to override them.

@gonfunko gonfunko requested a review from a team as a code owner October 31, 2024 19:11
@gonfunko gonfunko requested a review from cpcallen October 31, 2024 19:11
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

I admit that I am mildly concerned about arbitrarily increasing the size of our API, but this seems to be in aid of a good cause.

Is anything here breaking? If not, why not do this in v11? (If it is, then PR title needs updating.)

core/field_dropdown.ts Outdated Show resolved Hide resolved
core/field_dropdown.ts Outdated Show resolved Hide resolved
@gonfunko gonfunko changed the base branch from rc/v12.0.0 to develop November 4, 2024 19:12
@gonfunko
Copy link
Contributor Author

gonfunko commented Nov 4, 2024

I admit that I am mildly concerned about arbitrarily increasing the size of our API, but this seems to be in aid of a good cause.

Is anything here breaking? If not, why not do this in v11? (If it is, then PR title needs updating.)

Nothing here is breaking - there are no changes to behavior, it's just moving functions that were inaccessible into the class. I suppose in theory if somebody had a subclass that happened to have added methods with these names there would be an issue, but that feels like a stretch. Rebased to develop as well.

@gonfunko gonfunko closed this Nov 4, 2024
@gonfunko gonfunko reopened this Nov 4, 2024
@gonfunko gonfunko merged commit f1cbaab into google:develop Nov 8, 2024
8 checks passed
@gonfunko gonfunko deleted the field-dropdown branch November 8, 2024 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance FieldDropdown's extendability
2 participants