feat(frontend): full height component #3676
Conversation
Deploying windmill with
|
| Latest commit: |
2c5feed
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://65630b7d.windmill.pages.dev |
| Branch Preview URL: | https://full-height-component.windmill.pages.dev |
…abs/windmill into full-height-component
…abs/windmill into full-height-component
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 2f91838 in 2 minutes and 56 seconds
More details
- Looked at
936lines of code in15files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/component/Component.svelte:119
- Draft comment:
Ensure that the visibility logic correctly handles all application modes, not just 'preview'. If the intent is to hide components in other specific modes whenhiddenis true, this should be explicitly handled.
class="h-full flex flex-col w-full component {initializing ? 'overflow-hidden h-0' : ''} {hidden ? 'hidden' : ''} "
- Reason this comment was not posted:
Confidence of 50% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_BZ8B4zwrGCrdEfG7
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 2 days left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 396ed60 in 1 minute and 33 seconds
More details
- Looked at
28lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/GridViewer.svelte:101
- Draft comment:
The removal of dynamic visibility management (updateComponentVisibility) and settinghiddentofalsestatically might lead to performance issues or incorrect behavior if components overlap or are not meant to be visible. If the visibility logic is essential, consider reintroducing it or ensuring that the new approach handles these cases appropriately. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_gUjOxIUrlXVonfLk
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ffd6688 in 2 minutes and 31 seconds
More details
- Looked at
318lines of code in5files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/AppEditor.svelte:155
- Draft comment:
The removal ofsubGridMaxRowsfrom the context might affect features relying on this property. If this property is used elsewhere in the application, its removal should be handled carefully to avoid breaking existing functionalities. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_Jg6UqQ7sajLNXERB
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 957151d in 2 minutes and 6 seconds
More details
- Looked at
103lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/svelte-grid/MoveResize.svelte:316
- Draft comment:
The removal ofonlyHorizontalResizesimplifies the resizing logic to always allow both horizontal and vertical resizing. Ensure this change aligns with all intended use cases of the component. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_57BOicAbzti8xu3l
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on aa96b90 in 1 minute and 45 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/svelte-grid/MoveResize.svelte:330
- Draft comment:
Consider restoring the removed comment or providing a new, clear comment that explains the behavior and purpose of therepaintfunction when called withfalse, falseas arguments. This helps maintain code clarity and understandability. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_m0SBQSZiAfZzU8KG
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 58cd0b8 in 1 minute and 54 seconds
More details
- Looked at
60lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_k2e4805JxbJzKShg
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 018815e in 2 minutes and 12 seconds
More details
- Looked at
60lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_fov7BRncmY0va84j
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 7ae5e5d in 2 minutes and 11 seconds
More details
- Looked at
21lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/appUtils.ts:444
- Draft comment:
SettingfullHeighttofalseexplicitly when copying a component might contradict the intent of preserving component properties during duplication, especially since the PR is about handling full-height dynamically. Consider maintaining the originalfullHeightvalue from the component being copied.
item.data.fullHeight = item.data.fullHeight
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_Wr2PKMWAOqTgDp0r
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 4887f55 in 2 minutes and 8 seconds
More details
- Looked at
66lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/components/layout/AppContainer.svelte:17
- Draft comment:
The removal of theconfigurationprop fromAppContainerseems unintended as it is still being used inComponent.svelte. Please ensure that this change is intentional and correctly handled across all components. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_J6CcL4OmyMwWxWeJ
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on c2f07a6 in 1 minute and 44 seconds
More details
- Looked at
22lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/settingsPanel/ComponentPanel.svelte:448
- Draft comment:
The PR description mentions changes related toupdateComponentVisibilityandhiddenproperties, but these are not visible in the provided diff. Please ensure that these changes are correctly implemented where necessary. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_5cHK1wFsScIpKWdN
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 6a6526f in 3 minutes and 30 seconds
More details
- Looked at
177lines of code in8files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/settingsPanel/ComponentPanel.svelte:461
- Draft comment:
right: 'Mobile full height',
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_VlwwgiD1Mb4vFLtR
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3fa78c8 in 2 minutes and 29 seconds
More details
- Looked at
131lines of code in6files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/AppPreview.svelte:215
- Draft comment:
The removal ofrowHeightandgapproperties without replacing them with new constants or handling could potentially break the layout or functionality. Please ensure that these properties are correctly managed with the new full-height feature implementation.
This comment applies to multiple files including GridEditor.svelte, GridViewer.svelte, and SubGridEditor.svelte.
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_yIVSecba8ou2J1w8
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 38a4493 in 2 minutes and 14 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_mhw8j5lyoC5zLoNv
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
| @@ -81,17 +87,23 @@ | |||
| {@const width = | |||
| Math.min(getComputedCols, item[getComputedCols] && item[getComputedCols].w) * xPerPx - | |||
| gapX * 2} | |||
| {@const height = (item[getComputedCols] && item[getComputedCols].h) * yPerPx - gapY * 2} | |||
| {@const height = | |||
| ((breakpoint === 'lg' ? item.data['fullHeight'] : item.data['fullHeightMobile']) | |||
There was a problem hiding this comment.
It seems there might be a typo or incorrect property name used here. The PR description mentions 'fullHeightMobile', but the code references 'fullWidthMobile'. Please confirm and correct if necessary.
| ((breakpoint === 'lg' ? item.data['fullHeight'] : item.data['fullHeightMobile']) | |
| ((breakpoint === 'lg' ? item.data['fullHeight'] : item.data['fullHeightMobile']) |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 0d572c7 in 2 minutes and 15 seconds
More details
- Looked at
63lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/component/Component.svelte:129
- Draft comment:
It appears that thefullHeightproperty is not being handled in this component. Given the PR's goal to introduce a full-height feature, please ensure that thefullHeightproperty is appropriately integrated and utilized within the component logic. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_zj41KFCKXhtJsSfT
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 8b3f0af in 4 minutes and 30 seconds
More details
- Looked at
219lines of code in10files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/appUtils.ts:250
- Draft comment:
fullHeight: false // This line sets the fullHeight property to false when copying a component. If the intent is to preserve the original component's fullHeight state, this line should be adjusted.
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_z6pZWDuSuY0Cmw6V
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2c5feed in 5 minutes and 53 seconds
More details
- Looked at
453lines of code in9files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/AppPreview.svelte:232
- Draft comment:
fullHeight={dataItem?.[$breakpoint]?.fullHeight}
Note:
This issue is also present in multiple other files (GridEditor.svelte, GridViewer.svelte, SubGridEditor.svelte). The same correction should be applied there.
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
2. frontend/src/lib/components/apps/editor/GridEditor.svelte:146
- Draft comment:
fullHeight={dataItem?.[$breakpoint]?.fullHeight}
Note:
This issue is also present in multiple other files (GridViewer.svelte, SubGridEditor.svelte). The same correction should be applied there.
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
3. frontend/src/lib/components/apps/editor/GridViewer.svelte:91
- Draft comment:
(item?.[$breakpoint]?.fullHeight
Note:
This issue is also present in SubGridEditor.svelte. The same correction should be applied there.
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
4. frontend/src/lib/components/apps/editor/SubGridEditor.svelte:128
- Draft comment:
fullHeight={dataItem?.[$breakpoint]?.fullHeight}
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_EYn1LiKnJukg2si3
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
Screen.Recording.2024-05-14.at.15.36.59.mov
Summary:
Introduces a full-height component feature, allowing dynamic height adjustments to fill parent containers, enhancing layout flexibility.
Key points:
AppContainer.svelteto handle new full-height logicGenerated with ❤️ by ellipsis.dev
Summary:
This PR introduces a full-height feature for components, enhancing layout flexibility and dynamic height adjustments across multiple components and utility functions.
Key points:
fullHeightfeature across multiple frontend componentsfullHeightlogic in components likeAppContainer.svelte,AppEditor.svelte,AppPreview.svelte,GridEditor.svelte,GridViewer.svelte,ComponentPanel.svelte,Component.svelte,SubGridEditor.svelte,ComponentHeader.svelteGridViewer.svelteto update height calculation logic based onfullHeightpropertyGenerated with ❤️ by ellipsis.dev