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

Fix MarkdownDialog types #19703

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

davelopez
Copy link
Contributor

Fixes #19692

This is the easy fix. The real issue is here:

So the type defined in the event was incorrect:

(e: "onOk", results: Array<Record>): void;

The values returned depend on the format prop so the real type is certainly unknown.

We could still try to convert components/DataDialog/model.js to Typescript and try to do some Typescript black magic to define the proper type... react to this PR with:

  • 👍 if you think this is enough as a fix
  • or 🚀 if I should try to convert the file and use forbidden black magic

How to test the changes?

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

Fixes:
Type 'string | null' is not assignable to type 'string'.
Prop 'labels' requires default value to be set.
We don't really know what we are returning as the selected values can be arbitrarily returning different things depending on the `format` prop.
@davelopez davelopez added area/UI-UX kind/refactoring cleanup or refactoring of existing code, no functional changes labels Feb 25, 2025
@jmchilton
Copy link
Member

👍 because I think unblocking progress on Vue 3 is more important than typing the whole codebase correctly. In many ways it seems being stuck on Vue 2 is hurting our typing goal more than any individual change.

@github-actions github-actions bot added this to the 25.0 milestone Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI-UX kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typescript Error in MarkdownDialog
2 participants