Skip to content

fix(index.d.ts): fix getPixelWorldHeightAtCoord missing parameter#3236

Merged
finetjul merged 1 commit intoKitware:masterfrom
QQBoxy:master
Mar 28, 2025
Merged

fix(index.d.ts): fix getPixelWorldHeightAtCoord missing parameter#3236
finetjul merged 1 commit intoKitware:masterfrom
QQBoxy:master

Conversation

@QQBoxy
Copy link
Copy Markdown
Contributor

@QQBoxy QQBoxy commented Mar 27, 2025

Context

Currently, the getPixelWorldHeightAtCoord function in Sources/Widgets/Core/WidgetManager/index.js has a second parameter in its implementation, but the corresponding TypeScript definition in Sources/Widgets/Core/WidgetManager/index.d.ts is missing this parameter. This discrepancy may lead to type errors for TypeScript users.

Relevant code locations:

Results

This change updates the TypeScript definition to align with the implementation, preventing type errors when using getPixelWorldHeightAtCoord.

Before Change

getPixelWorldHeightAtCoord(coord: number[]): number;

After Change

getPixelWorldHeightAtCoord(coord: number[], displayScaleParams: IDisplayScaleParams): number;

Changes

  • Fixed the function definition in index.d.ts to include the missing second parameter.
  • Ensured TypeScript definitions match the JavaScript implementation.

PR and Code Checklist

  • Followed semantic-release commit message guidelines.
  • Ran npm run reformat to ensure proper code formatting.

Testing

  • Just fill in the missing parameters, there should be no impact, or you can use function overload.
  • Tested Environment:
    • vtk.js: Latest master
    • OS: MacOS
    • Browser: Chrome 134.0.6998.166

Copy link
Copy Markdown
Member

@finetjul finetjul left a comment

Choose a reason for hiding this comment

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

Please squash your commits and follow https://www.conventionalcommits.org/ style (i.e. use npm run commit)

Comment thread Sources/Widgets/Core/WidgetManager/index.d.ts Outdated
Comment thread Sources/Widgets/Core/WidgetManager/index.d.ts
@finetjul
Copy link
Copy Markdown
Member

I use npm run commit and rebase the commit. Is that correct?

I think it is correct.
However, "npm run reformat" should not appear in the commit message. And the line "fix(index.d.ts): fix getPixelWorldHeightAtCoord missing parameter" should be the "title" of your commit (i.e. instead of "Update index.d.ts")

@QQBoxy QQBoxy changed the title Fix getPixelWorldHeightAtCoord missing parameter [Bug] Fix getPixelWorldHeightAtCoord missing parameter Mar 28, 2025
Copy link
Copy Markdown
Member

@finetjul finetjul left a comment

Choose a reason for hiding this comment

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

LGTM

@QQBoxy QQBoxy changed the title [Bug] Fix getPixelWorldHeightAtCoord missing parameter fix(index.d.ts): fix getPixelWorldHeightAtCoord missing parameter Mar 28, 2025
@finetjul finetjul added this pull request to the merge queue Mar 28, 2025
Merged via the queue into Kitware:master with commit 22b4dce Mar 28, 2025
2 checks passed
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 32.12.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions Bot added the released Automated label label Mar 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released Automated label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants