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

feat: use unique identifier for actions #366

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

dpilch
Copy link
Member

@dpilch dpilch commented Jan 26, 2022

If you have any suggestions on how the unique identifier should be constructed please leave a comment. I just went with one structure I though was sensible.

I needed to move some functions to a new file to avoid a cyclical dependency between action.ts and react-component-render-helper.ts.

@dpilch dpilch requested a review from a team January 26, 2022 18:31
@@ -67,7 +57,7 @@ export class ReactComponentRenderer<TPropIn> extends ComponentRendererBase<
buildOpeningElementProperties(value, key),
);
const eventAttributes = Object.entries(this.component.events || {}).map(([key, value]) =>
buildOpeningElementEvents(value, mapGenericEventToReact(key as StudioGenericEvent)),
Copy link
Member Author

@dpilch dpilch Jan 26, 2022

Choose a reason for hiding this comment

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

Pushed mapGenericEventToReact further down so that we would get the same result form getActionIdentifier.

@alharris-at
Copy link
Contributor

I think to reduce the wordiness of this, we may be able to just generate the action name from the component name and action, since that combination should be unique. The action type probably doesn't need be put in the variable, since it's referenced immediately after in the declaration. That's also easy enough to change later, so this overall LGTM.

@dpilch
Copy link
Member Author

dpilch commented Jan 26, 2022

Good idea, I will update to this approach.

@dpilch dpilch merged commit 39d1ada into tagged-release/q1-release Jan 27, 2022
@dpilch dpilch deleted the workflow/unique-identifier branch January 27, 2022 17:41
@dpilch dpilch mentioned this pull request Feb 24, 2022
@alharris-at alharris-at mentioned this pull request Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants