doc: Update README.md #33

stefanoamorelli · 2025-01-24T16:04:58Z

No description provided.

SkoebaSteve · 2025-01-24T16:29:42Z

At the top of the file there's a link to a banner image:

    
           ![ember-autofocus-modifier-illustration](https://gitlab.qonto.co/npm-packages/react-migration-toolkit/uploads/c0af9dd2fe40080b5e340a35f319147e/react-migration-toolkit-banner.png)

which is broken now. It only worked in our own github repo.

It's also broken on npmjs (checked in incognito mode):

stefanoamorelli · 2025-01-27T10:16:00Z

At the top of the file there's a link to a banner image:

react-migration-toolkit/README.md

Line 1 in 06e9dff

![ember-autofocus-modifier-illustration](https://gitlab.qonto.co/npm-packages/react-migration-toolkit/uploads/c0af9dd2fe40080b5e340a35f319147e/react-migration-toolkit-banner.png)

which is broken now. It only worked in our own github repo.

It's also broken on npmjs (checked in incognito mode):

That's removed ✔️

poulet42 · 2025-01-27T13:16:48Z

README.md

+
+Additional providers might be needed for your app. For example, the toolkit provides the following providers and associated hooks:
+
+- `RawIntlProvider` (for `react-intl`) → `useIntl` (requires passing the Ember `intl` instance from `useEmberIntl`)


It's not provided by the package per se (at lest not currently), consumers of the addon need to install react-intl and pass the result of useEmberIntl themselves to RawIntlProvider, which is coming the react-intl package.

poulet42 · 2025-01-27T13:18:42Z

README.md

+
+#### `providersOptions` (optional)
+
+To have access to the Launch Darkly feature flags in react components, use `ReactBridgeWithProviders` instead of `ReactBridge`, and in the react component you will then be able to use the flags hook.


there's no ReactBridgeWithProviders in this package, also I feel like this text is focusing too much on LD / flags. It's unclear what providerOptions mean here

poulet42 · 2025-01-27T13:19:40Z

README.md

+<ReactBridgeWithProviders
+  @reactComponent={{this.TransactionsSidebarFooter}}
+  @props={{hash
+    highlightedItem=@highlightedItem
+    handleShowHelpSection=@handleShowHelpSection
+    className=(local-class "footer")
+   }}
+  />


same as above, this component is created (optionally) on the consumer's side, it's not provided here

poulet42 · 2025-01-27T13:23:27Z

README.md

+In the React component you can use the `useFlags` hook imported from `@qonto/react-migration-toolkit/react/hooks` which is accessible from the flags object (in camelCase)

-It renders React components within Ember templates, permitting progressive UI migration and preserving existing logics and tests.
+Example:

-Full documentation can be found [here](https://www.notion.so/qonto/How-to-use-React-components-in-Ember-applications-e2f8513fc4a442b0967274d9e57b18b0) (Qonto VPN must be on).
+```jsx
+const { featurePrismicZendeskMigration } = useFlags()
+```


doesn't seem related to the section, and maybe we should remove stuff about LD for now

poulet42 · 2025-01-27T13:30:32Z

README.md

+|---------------------|-------------------------------------------|----------|---------|
+| reactComponent      | ComponentType                            | Yes      | None    |
+| props               | object                                   | No       | None    |
+| providersComponent  | ComponentType<{ children: ReactNode } & P> | No       | None    |


doesn't seem right, in the current state of things, this should be providerOptions with component and props inside

poulet42 · 2025-01-27T13:32:03Z

README.md

+const { featurePrismicZendeskMigration } = useFlags()
+```
+
+#### `component` prop


it looks unclear where this is coming from here, this is not really a "prop" and it's one of the keys of the providerOptions argument

poulet42 · 2025-01-27T13:33:25Z

README.md

+
+- `RawIntlProvider` (for `react-intl`) → `useIntl` (requires passing the Ember `intl` instance from `useEmberIntl`)
+- `PolymorphicRouterContextProvider` → `useRouter`
+- `LDProvider` (LaunchDarkly) → `useFlags`


there's more setup to do on the apps side to make LDProvider works, it's just easier if we don't document it for now

poulet42 · 2025-01-27T13:34:51Z

README.md


-[This branch](https://gitlab.qonto.co/front/qonto-js/-/blob/bifrost/app/components/react-bifrost.hbs) uses the ReactBridge extensively and can be checked for reference.
+#### `props` prop


same as the "component" comment, I think it's not super clear

poulet42 · 2025-01-27T13:36:30Z

README.md

+
+#### `tagName` prop
+
+By default, the bridge will wrap the React component in a `div` which sometimes this breaks the semantic flow. For example, a `table` can not have a `div` element as a child. With this, we can to set another tag dynamically to wrap the react component which solves this problem.


Suggested change

By default, the bridge will wrap the React component in a `div` which sometimes this breaks the semantic flow. For example, a `table` can not have a `div` element as a child. With this, we can to set another tag dynamically to wrap the react component which solves this problem.

By default, the bridge will wrap the React component in a `div` which sometimes breaks the semantic flow. For example, a `table` cannot have a `div` element as a child. With this, we can set another tag dynamically to wrap the react component which solves this problem.

some typos here

poulet42 · 2025-01-27T13:39:19Z

README.md

+import { useEmberService } from '@qonto/react-migration-toolkit/react/hooks';
+
+export const useOrganizationManager = () => {
+  return useEmberService('organization-manager');


can we use a more generic service name here?

poulet42 · 2025-01-27T13:40:19Z

README.md

+
+To make sure the correct types is available when a service is injected with `useEmberService` hook, the service should be declared in Ember’s dependency injection registry which will be then used through [declaration merging](https://www.typescriptlang.org/docs/handbook/declaration-merging.html).
+
+```tsx


nit: this is a typescript file, not tsx

poulet42 · 2025-01-27T13:42:22Z

README.md

+
+The same API of the Ember service is available after exposing it via the hook.
+
+To make sure the correct types is available when a service is injected with `useEmberService` hook, the service should be declared in Ember’s dependency injection registry which will be then used through [declaration merging](https://www.typescriptlang.org/docs/handbook/declaration-merging.html).


this is optional, and I would use a wording closer to what's in the official docs here (check the last section)

poulet42 · 2025-01-27T13:43:16Z

README.md

+<details>
+<summary>How to test React components in Ember</summary>
+
+Testing for React is currently done via **QUnit** and existing Ember test suites should still largely be compatible with migrated components. Changing framework should not impact components behavior.


the way this is phrased, this sounds more like an internal note than public documentation

doc: add LICENSE.md file

stefanoamorelli self-assigned this Jan 24, 2025

stefanoamorelli marked this pull request as draft January 24, 2025 16:05

stefanoamorelli requested a review from SkoebaSteve January 24, 2025 16:05

stefanoamorelli force-pushed the doc/update-readme branch from 0c3c338 to 06e9dff Compare January 24, 2025 16:07

stefanoamorelli force-pushed the doc/update-readme branch from 06e9dff to cccb79a Compare January 27, 2025 10:14

stefanoamorelli requested a review from poulet42 January 27, 2025 10:16

stefanoamorelli force-pushed the doc/update-readme branch 2 times, most recently from c932030 to 108b52f Compare January 27, 2025 10:50

poulet42 reviewed Jan 27, 2025

View reviewed changes

stefanoamorelli force-pushed the doc/update-readme branch 3 times, most recently from f68ef96 to 83ad31c Compare January 27, 2025 16:48

doc: update README.MD

11fccd4

doc: add LICENSE.md file

stefanoamorelli force-pushed the doc/update-readme branch from 83ad31c to 11fccd4 Compare January 27, 2025 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: Update README.md #33

doc: Update README.md #33

stefanoamorelli commented Jan 24, 2025

SkoebaSteve commented Jan 24, 2025 •

edited

Loading

stefanoamorelli commented Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025

poulet42 Jan 27, 2025


		Additional providers might be needed for your app. For example, the toolkit provides the following providers and associated hooks:

		- `RawIntlProvider` (for `react-intl`) → `useIntl` (requires passing the Ember `intl` instance from `useEmberIntl`)


		#### `providersOptions` (optional)

		To have access to the Launch Darkly feature flags in react components, use `ReactBridgeWithProviders` instead of `ReactBridge`, and in the react component you will then be able to use the flags hook.


		[This branch](https://gitlab.qonto.co/front/qonto-js/-/blob/bifrost/app/components/react-bifrost.hbs) uses the ReactBridge extensively and can be checked for reference.
		#### `props` prop


		#### `tagName` prop

		By default, the bridge will wrap the React component in a `div` which sometimes this breaks the semantic flow. For example, a `table` can not have a `div` element as a child. With this, we can to set another tag dynamically to wrap the react component which solves this problem.


		To make sure the correct types is available when a service is injected with `useEmberService` hook, the service should be declared in Ember’s dependency injection registry which will be then used through [declaration merging](https://www.typescriptlang.org/docs/handbook/declaration-merging.html).

		```tsx


		The same API of the Ember service is available after exposing it via the hook.

		To make sure the correct types is available when a service is injected with `useEmberService` hook, the service should be declared in Ember’s dependency injection registry which will be then used through [declaration merging](https://www.typescriptlang.org/docs/handbook/declaration-merging.html).

doc: Update README.md #33

Are you sure you want to change the base?

doc: Update README.md #33

Conversation

stefanoamorelli commented Jan 24, 2025

SkoebaSteve commented Jan 24, 2025 • edited Loading

stefanoamorelli commented Jan 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SkoebaSteve commented Jan 24, 2025 •

edited

Loading