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: add missing type definitions #659

Conversation

lkritsimas
Copy link
Collaborator

@lkritsimas lkritsimas commented Jun 14, 2024

Describe pull-request

Added type definitions that were missing when using modern TypeScript moduleResolution options, such as bundler.

Issue Linking:

How to test

Provide detailed steps for testing, including any necessary setup.

  1. Go to...
  2. Check in...
  3. Run ...

Checklist before submission

  • I have added unit tests for my changes (if applicable)
  • All existing tests pass
  • I have updated the documentation (if applicable)

Suggested test steps

  • Browser testing (Chrome, Safari, Firefox)
  • Keyboard operability
  • Interactive elements have labels.
  • Storybook controls
  • Design/controls/props is aligned with other components
  • Dark/light mode and variants
  • Input fields – values should be displayed properly
  • Events

Screenshots

Include before/after screenshots for UI changes.

Additional context

I wasn't able to run the tests locally because of issues with Docker/WSL on my work laptop.

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-659.d3fazya28914g3.amplifyapp.com

Copy link
Collaborator

@mJarsater mJarsater left a comment

Choose a reason for hiding this comment

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

Nice, could we make sure we do the same for the angular and core packages?

@lkritsimas
Copy link
Collaborator Author

lkritsimas commented Jun 17, 2024

Nice, could we make sure we do the same for the angular and core packages?

This is a change in the core package so it should apply to all packages that depend on it.

The issue that I created may be a bit misleading in hindsight. This should apply to any typescript project using modern module resolution options, however I only tried it out in a React+Vite project.

Copy link
Contributor

@theJohnnyMe theJohnnyMe left a comment

Choose a reason for hiding this comment

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

Nice contribution! Thanks @lkritsimas !

@theJohnnyMe theJohnnyMe merged commit ba5badd into scania-digital-design-system:develop Jun 19, 2024
2 checks passed
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.

4 participants