[WIP] Fixes #26078 - interface table in react

[ezr-ondrej]

I have tried to unwrap the mess we got in the host form.
This could have been the first step. I tried to be not-so intrusive and keep as much as possible untouched, but with bringing some improvements.
Interfaces table is already react component, but otherwise form stays the same.
The interfaces data are stored in redux and all the changes are made through redux action calls.

Changes are written to the interfaces hidden forms by observing the store.

To consider:

• breaks possibility of extending the table ( addressed by: [WIP] Fixes #25756 - Adds extend-ability to react components #6366 )

Any comments are welcome.

[theforeman-bot]

Issues: #26078

[ares]

In general, I like the direction. The interface form is extended by REX plugin, so we'll need to wait until slot/fill is done. But I suppose @theforeman/ui-ux or anyone else can start looking already :-)

[amirfefer]

@ezr-ondrej Thanks for this great contribution 👍

few general feedback:

1. @boaz0 's implementation for patternfly tables is almost done, I think it would be better to use it
2. Please use our folder structure
3. We need to have a better way for invoking legacy functions in react component, IMHO this approach
   (using the window object for calling functions) mix the react with legacy in a way that bugs might occur and makes it harder to understand and maintain, especially if we wish to deprecate parts of it someday.
   Using redux can be a solution, so a react component can invoke an action that change a specific part of the store, which a legacy function can observe. with this approach we can keep our react code and legacy code isolated, but still affect each other
4. we do have a re-design task for the host creation in our roadmap (Host creation wizard), however I think it should be planned well before the implementation, according to up to date mocks and user stories.

[amirfefer]

Maybe I missed something but is it necessary to unmount the component ? how about changing its data and by that forcing re-render?

[ezr-ondrej]

It is not necessary, but the dom is going to be gone, after this method is done.
So I thought it would be nice to clean up.

[amirfefer]

if the dom is going to be gone, unmount will be invoked automatically here

[amirfefer]

this object should be bind, you can use bindMethods helper function

[ezr-ondrej]

Are you sure? I thought it is necessary just in callbacks, this is not callback, it is ordinary call.

[amirfefer]

Please split the component logic and the redux wrapper.
The component's folder strucutre should look like:

webpack/assets/javascripts/react_app/components/<Compoment-name>
├── <Compoment-name>Actions.js
├── <Compoment-name>Reducer.js
├── <Compoment-name>.fixtures.js 
├── <Compoment-name>.scss.js // if needed
├── <Compoment-name>.js
├── index.js // the redux connected file 
└── __tests__
    ├── <Compoment-name>Actions.test.js
    ├── <Compoment-name>Reducer.test.js
    ├── <Compoment-name>.test.js
    ├── integration.test.js
    └── __snapshots__
        ├── All snapshot files (created automatlicy by `npm test -- -u`)

for a good example you can have a look on Layout or BreadcrumbsBar components.

[amirfefer]

it looks like an anti-pattern, note that this element is a dom element, while react uses the virtual dom.
so mixing these elements inside a react component may lead to some errors (for instance, the real dom element could not be exist while the component is re-rendered).

IMO we need to find a better solution for calling legacy functions inside a react component, because with this way it would much more complicated remove old code.

[waldenraines]

I agree, this is an anti-pattern and creates technical debt. I like @amirfefer's suggestion of using redux to communicate between jquery and react.

[ezr-ondrej]

well, I can clearly see that it is an anti-pattern :) I just am not sure what to do about it, I can shove it in the webpack, to look more clean. But the method is in the window, because the technical debt from past. I am all for moving it in the webpack, but it will throw away the support to be extendible by more providers the same way it was - basically it will throw the API support. And I am not sure till what level we can do that 🤔 I was thinking about some deprecations, but I don't know what is the best way to do this thinks in the future.
I will try to talk about it with @amirfefer, but I don't feel like it should be addressed in this PR.

[amirfefer]

ditto

[ezr-ondrej]

solved through setting up the interface editing property and if that is true, we open the modal window from the legacy code.

[iNecas]

FYI @ezr-ondrej I guess this is the table PR @amirfefer had in mind

[Ron-Lavi]

what's the status of this PR? thanks

[ezr-ondrej]

what's the status of this PR? thanks

Still in WIP. But if @amirfefer, @LaViro or/and @sharvit would like to have serious discussion about this one next week, I have some time on Friday and I could push it to a "hackable" state. Just ping me if that is the case :)

[tbrisker]

closing following discussion with @ezr-ondrej. Thanks everyone!