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

React - override id, component; convert Breadcrumbs to functional #7070

Merged
merged 8 commits into from
May 22, 2020
Merged

React - override id, component; convert Breadcrumbs to functional #7070

merged 8 commits into from
May 22, 2020

Conversation

himdel
Copy link
Contributor

@himdel himdel commented May 21, 2020

(Split off from #6963 as it was getting too large.)

This allows for overriding react component ids when calling = react,
overriding component registry components from plugins via addReact(name, component, { override: true }),
adds a webpack plugin to output timestamp of the last build (scss rebuilds take a few seconds),

and converts Breadcrumbs to a functional component, without further changes

Cc @skateman

without this, ids get silently ignored for =react
    ManageIQ.component.addReact('Foo', Foo); // works
    ManageIQ.component.addReact('Foo', Bar); // throws
    ManageIQ.component.addReact('Foo', Baz, { override: true }); // works
previously, the third param was an array of instances,
now it's an options object, with instances an optional key

and attempting to add a numeric name, or a component twice without override throws
switch from Map<{name, blueprint}, Set<instances>>
to {name: {name, blueprint, instances: Set}}

That way, we ensure a component is only registered once under a name.

And removed the surprising "instance.id must be a string" logic.
needed for now, later fixing in css
@miq-bot
Copy link
Member

miq-bot commented May 22, 2020

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/381b955c73c8ca6dbca01d6f1cacc0bf744ae3e6~...12f59ff9363e8474640b0fcc2388e3a652756799 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@skateman skateman merged commit d4a1b0d into ManageIQ:master May 22, 2020
@himdel himdel deleted the react-override branch May 22, 2020 14:49
@himdel himdel mentioned this pull request May 26, 2020
simaishi pushed a commit that referenced this pull request May 28, 2020
React - override id, component; convert Breadcrumbs to functional

(cherry picked from commit d4a1b0d)
@simaishi
Copy link
Contributor

Jansa backport details:

$ git log -1
commit 58edd4ec0adb524110420fc25f203e5e0eb01272
Author: Halász Dávid <skateman@users.noreply.github.com>
Date:   Fri May 22 16:49:49 2020 +0200

    Merge pull request #7070 from himdel/react-override

    React - override id, component; convert Breadcrumbs to functional

    (cherry picked from commit d4a1b0d44009508ebbf3600c41950d93b9611d7d)

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.

5 participants