feat: action redesign, UQI upgrade S3 plugin config to dual zone format & sorting field responsiveness #36002
Conversation
github-actions
bot
added
IDE Navigation
WalkthroughThe recent changes involve significant updates to the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
Early access features: disabledWe are currently testing the following features in early access:
Note:
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10629833514. |
alex-golovanov
requested review from
hetunandu,
ankitakinger and
albinAppsmith
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/src/components/formControls/SortingControl.tsx (4 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/root.json (1 hunks)
Additional context used
Biome
app/client/src/components/formControls/SortingControl.tsx
[error] 156-156: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 180-190: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Additional comments not posted (12)
app/client/src/components/formControls/SortingControl.tsx (7)
77-77: Type Definition Update:SortingControlPropsThe update to
SortingControlPropsto directly extendControlPropssimplifies the type hierarchy and ensures that all properties fromControlPropsare available, enhancing type safety and clarity.
79-80: Type Definition Update:SortingComponentPropsThe new definition for
SortingComponentPropscombinesWrappedFieldArrayPropswith selected properties fromSortingControlProps. This change is beneficial as it narrows down the properties to what is actually used in the component, improving type safety and clarity.
55-60: Updated Layout Using CSS GridThe transition from a flexbox to a grid system in
SortingContainerallows for more responsive design adjustments. This is a positive change as it enhances the layout's adaptability to different screen sizes.
63-68: Responsive Design inSortingFieldsThe use of CSS grid in
SortingFieldsfor responsive layout adjustments is commendable. It improves the component's responsiveness and aligns with modern web design practices.
112-130: Refinement of Logic inuseEffectHookThe logic within the
useEffecthook has been simplified by directly checkingsortObjectValuebefore proceeding with conditional checks for pushing new fields. This streamlining reduces unnecessary complexity and enhances the readability and maintainability of the component.
90-90: Direct Reference inonDeletePressedFunctionThe update to use
fields.remove(index)directly instead ofprops.fields.remove(index)aligns with the updated props structure and simplifies the function's implementation.
155-191: Rendering of Sorting Fields with Responsive DesignThe rendering logic for sorting fields now utilizes the new
SortingFieldsstyled component, incorporating responsive design principles. This change is beneficial as it ensures the UI adapts well to different screen sizes.Tools
Biome
[error] 156-156: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 180-190: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/root.json (5)
4-4: Transition toSECTION_V2Control TypeThe upgrade from
SECTIONtoSECTION_V2enhances the organization and functionality of the editor by allowing for more complex interactions and configurations. This change is crucial for supporting the new dual zone format and improving the user interface.
9-53: Addition ofDOUBLE_COLUMN_ZONEThe introduction of
DOUBLE_COLUMN_ZONEfacilitates a more sophisticated layout for user inputs. This change is significant as it allows for a clearer separation of controls and enhances the overall user experience by organizing related settings more intuitively.
56-69: Conditional Visibility inDOUBLE_COLUMN_ZONEThe use of conditional logic to show or hide zones based on user selections is a smart addition. It streamlines the user experience by only displaying relevant options, reducing clutter and potential confusion.
71-109: Enhanced File Operation ConfigurationsThe new fields for generating signed and unsigned URLs, complete with options for expiration durations, reflect a more comprehensive approach to managing file operations. This enhancement provides users with greater flexibility and control, improving the functionality of the S3 plugin.
111-245: Expanded Command Options and Conditional FieldsThe expansion of command options and the addition of conditional fields for various file operations like uploading and deleting multiple files enhance the plugin's capabilities. These changes make the interface more robust and user-friendly by catering to a wider range of user needs.
|
Deploy-Preview-URL: https://ce-36002.dp.appsmith.com |
hetunandu
requested review from
AmanAgarwal041 and
sneha122
and removed request for
hetunandu
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
app/client/src/components/formControls/SortingControl.tsx (1)
1-9: Reminder: Address TODO items.The TODO comments suggest improvements for type safety in future edits. It's important to address these soon to maintain code quality and avoid technical debt.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/src/components/formControls/SortingControl.tsx (4 hunks)
Additional comments not posted (4)
app/client/src/components/formControls/SortingControl.tsx (4)
82-131: Good simplification of logic.The refactoring within
SortingComponentenhances readability and reduces complexity. Ensure that direct manipulations of Redux state, likefields.remove(index), are properly managed to avoid state inconsistencies.
220-230: Efficient use of React hooks.The use of
useMemofor optimizing props passed toFieldArrayis a good practice, ensuring that the component only re-renders when necessary.
77-80: Enhanced type safety.Updating
SortingControlPropsandSortingComponentPropsto be more specific improves clarity and helps prevent type-related bugs.
55-68: Responsive layout improvements.Switching to a grid layout and using
isBreakpointSmallfor responsive adjustments enhances the UI's adaptability across different devices.
alex-golovanov
changed the title
Description
Upgrade S3 plugin config to new format using SECTION_V2, SINGLE_COLUMN_ZONE, and DOUBLE_COLUMN_ZONE.
Fixes #35484
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10662975214
Commit: 1905f4d
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ServerSide/ApiTests/API_All_Verb_spec.js
List of identified flaky tests.Mon, 02 Sep 2024 08:57:59 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes