-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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 zone elevatedBackground evaluated values and upgrade space distribution ux of borderless zones. #33527
Conversation
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9114387483. |
Deploy-Preview-URL: https://ce-33527.dp.appsmith.com |
Warning Rate Limit Exceeded@marks0351 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 37 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates aim to enhance widget elevation and space distribution features in the Anvil layout system. These changes introduce 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9125851763. |
Deploy-Preview-URL: https://ce-33527.dp.appsmith.com |
@@ -37,11 +44,13 @@ export function useWidgetBorderStyles(widgetId: string, widgetType: string) { | |||
if (isPreviewMode) { | |||
return {}; | |||
} | |||
|
|||
const isBorderLessZoneOfDistributingSection = |
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 don't quite understand semantics of this variable, could you rename it or add comment?
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.
shoot. It should have been isBackgroundLessZoneOfDistributingSection to show border for those zones belonging to the current section that is distributing space and also whose backgorund is switched off, let me break this to two constants to make it more readable.
useContext, | ||
} from "react"; | ||
|
||
interface WidgetElevationObj { |
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.
interface WidgetElevationObj { | |
interface WidgetElevation { |
…o fix/zone-elevation
…o fix/zone-elevation
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9153933334. |
Deploy-Preview-URL: https://ce-33527.dp.appsmith.com |
…o fix/zone-elevation
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9171886636. |
Deploy-Preview-URL: https://ce-33527.dp.appsmith.com |
…o fix/zone-elevation
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9172271481. |
Deploy-Preview-URL: https://ce-33527.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9172567279. |
Deploy-Preview-URL: https://ce-33527.dp.appsmith.com |
Description
In this PR we are making sure the evaluated values of elevatedBackground(prop that indicates if a section or zone if elevated) is used instead of the unevaluated value used so far.
For this we will need to refer to siblings as well, so fetching the data tree state and iterating to find all siblings is going to be underperformant.
Hence, creating a context for the editor alone which will collect all sections and zones current evaluated elevated background.
This context will be accessed by
useAnvilDnDListenerStates
anduseAnvilDnDCompensators
to decide compensators for a zone and section.We have also enhanced space distribution UX.
Fixes #33369
Fixes #33212
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Anvil"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9172567552
Commit: 40a6bca
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?