Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
chore: [Plugin Action Editor] Combine Plugin Editor UI state #36651
chore: [Plugin Action Editor] Combine Plugin Editor UI state #36651
Changes from all commits
2e624d0
58cc2c8
966b8c5
e776758
3da3aef
ce4ba25
f9a81ba
95a7e8f
58a9158
151246e
da1a8ee
d08e3ad
08d621b
25f3629
d74ba51
f439f39
eee09c8
b1683d0
d1da237
14517e9
3a204e2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
store
actions
reducer
selector
^ is this folder pattern we are establishing to use/refactor consistently everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. There are so many other flows which we are not handling in Action redesign (like admin settings), won't the folder structure for those be different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to make sure the store is near the feature and this makes sense to me. This would be a gradual change as we start making more modular features. @ankitakinger can you share why this does not work ? Please suggest any alternatives you can think of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not saying it won't work. My point is about consistency, since we are not going to touch other flows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to Hetu, much easier to consume and refactor components if all related items in a domain level package. One question though, why not bring sagas into this? Also, let's publish this expectation to wider team also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider Adding Explicit Return Types to Exported Functions for Clarity
It's beneficial to explicitly specify return types for your exported functions. This practice enhances code readability and type safety, making it clearer for others (and your future self) to understand what each function returns. It can also help catch unintended type errors during development.
For example, you can define the return type of
getActionEditorSavingMap
asRecord<string, boolean>
:📝 Committable suggestion