Skip to content

refactor(control-custom-state): fix misleading button variable names - #1734

Open
zigzagdev wants to merge 1 commit into
googlemaps-samples:mainfrom
zigzagdev:refactor/control-custom-state-button-naming
Open

zigzagdev wants to merge 1 commit into
googlemaps-samples:mainfrom
zigzagdev:refactor/control-custom-state-button-naming

Conversation

@zigzagdev

Copy link
Copy Markdown

Summary

The two button variable names in samples/control-custom-state/index.ts were swapped relative to their actual behavior. setCenterButton was bound to btnCenterMap (the button that recenters the map) but only applies the stored center, so it doesn't "set" anything.
resetCenterButton was bound to btnSetCenter (the button that stores the current center) but is the one that actually stores it, so it doesn't "reset" anything.

What I have done

  1. Renamed setCenterButton to centerMapButton (bound to btnCenterMap)
  2. Renamed resetCenterButton to setCenterButton (bound to btnSetCenter)
  3. Updated only identifiers to match the HTML ids/labels and what each click handler actually does. No behavior change.

Test Plans

  1. Run npx tsc --noEmit to confirm the sample still type-checks
  2. Confirm no HTML changes were needed since btnCenterMap and btnSetCenter ids are unchanged

Test Results

  1. npx tsc --noEmit passed with no errors

The variables were swapped relative to their actual behavior and the
HTML ids/labels they're bound to: setCenterButton was wired to
btnCenterMap and recentered the map, while resetCenterButton was wired
to btnSetCenter and stored the current center. Renamed to
centerMapButton/setCenterButton to match what each one actually does.

@willum070 willum070 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great suggestion, thanks!

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