chore(ui): migrate AppShellProvider and CodeBlock to TypeScript

[gjaskiewicz]

Summary

Migrate AppShellProvider and CodeBlock to TypeScript

Changes Made

• migrate AppShellProvider to TypeScript
• migrate CodeBlock to TypeScript
• expose deprecated_js version of AppShellProvider in index.js - temporary fix until this component is removed
• add replacement for CodeBlock in stories (to be replaced after migration of this component)

Related Issues

• Issue 1: [link to issue]
• Issue 2: [link to issue]

Testing Instructions

1. npm i
2. npm run storybook

Checklist

• I have performed a self-review of my code.
• I have commented my code, particularly in hard-to-understand areas.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes.
• I have made corresponding changes to the documentation (if applicable).
• My changes generate no new warnings or errors.

[changeset-bot]

🦋 Changeset detected

Latest commit: 21352ac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@cloudoperators/juno-ui-components	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-11-05 23:44 UTC

[barsukov]

Please fix the Andreas comments and also from my side change index.js to use a deprecated one.

When we migrate Combobox and also Select we will delete all deprecated related files and change the exports in the index.js

[barsukov]

We close caause we need first to migrate the jsonviewer component and also codeblock

[barsukov]

few comments

[gjaskiewicz]

(Requested Change)

The AppShellProvider has a few breaking changes 🐞:

See screenshots (JS code on left, TS on right)

• Setting theme-dark and theme-light is broken.
• theme Story control has changed from string to object.
• theme type has changed from string to string|null. If the theme can only be theme-light and theme-dark, let's create a custom type containing these (instead of just string)?
• theme default value should be theme-dark?

It seems like a potential breaking change in apps to me. If type discrepency is problem in storybook I could just change a control type there. Otherwise it works when theme-light or theme-dark is entered with quotes.

[guoda-puidokaite]

@gjaskiewicz Nice that it's just a Storybook issue. I think that the storybook documentation should stay unchanged though after the migration. At least to me, it's confusing why I can't just enter text and would have to use quotes, since type is only string|null. Please also consider the other suggestions regarding custom type and type default, I think it would be an appropriate time for the improvements. 🚀

[edda]

@gjaskiewicz Nice that it's just a Storybook issue. I think that the storybook documentation should stay unchanged though after the migration. At least to me, it's confusing why I can't just enter text and would have to use quotes, since type is only string|null. Please also consider the other suggestions regarding custom type and type default, I think it would be an appropriate time for the improvements. 🚀

Agreed in this case. It is not a breaking change if we ensure that by default the prop is set to "theme-dark". It's also more correct and helps people understand what is happening under the hood.

[gjaskiewicz]

@gjaskiewicz Nice that it's just a Storybook issue. I think that the storybook documentation should stay unchanged though after the migration. At least to me, it's confusing why I can't just enter text and would have to use quotes, since type is only string|null. Please also consider the other suggestions regarding custom type and type default, I think it would be an appropriate time for the improvements. 🚀

I removed null value as it resolves to "theme-dark" anyway.

[guoda-puidokaite]

(Improvement)

For AppShellProvider, the children type is an array in Storybook but React.ReactNode in the code.

I think we could improve this for AppShellProvider (left old .JS code, right new .TSX code) :

In other components, it's any, as Storybook doesn't have an appropriate type with a comment.
Example of what we do in Message:

[guoda-puidokaite]

Small comment on types in Storybook, but idk if necessary to change.
Otherwise Thank You for the type changes! Looks great. 🚀💪🤪

[barsukov]

I left one comment that kinda nice to fix before we merge.

[barsukov]

LGTM from my side good job 🥇

[guoda-puidokaite]

LGTM too! 🚀 Thank You!

Vacation time, the issues are addressed and fixed.