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(frontend): Add context for inner components of list, groups and … #3974

Merged
merged 13 commits into from
Jun 26, 2024

Conversation

fatonramadani
Copy link
Contributor

@fatonramadani fatonramadani commented Jun 25, 2024

Add missing contexts table actions and sub components
Screenshot 2024-06-25 at 15 27 24
Screenshot 2024-06-25 at 15 27 41
…tables


🚀 This description was created by Ellipsis for commit 8116e30

Summary:

Enhanced handling of context variables in the settings panel, including updates to logic and the introduction of a function to prevent duplicates, along with the removal of an outdated function.

Key points:

  • Added context variables for inner components of lists, groups, and tables in frontend/src/lib/components/apps/editor/settingsPanel/ContextVariables.svelte
  • Introduced addContextVariable function to avoid duplicate entries
  • Updated logic to handle various component types including containercomponent, listcomponent, and various table-related components
  • Handled cases for multiple component types by determining parent component type and adding appropriate context variables
  • Removed findEveryParentGridItem function from frontend/src/lib/components/apps/editor/appUtils.ts

Generated with ❤️ by ellipsis.dev

Copy link

cloudflare-pages bot commented Jun 25, 2024

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 057c0da
Status: ✅  Deploy successful!
Preview URL: https://700ca92d.windmill.pages.dev
Branch Preview URL: https://fr-add-missing-context.windmill.pages.dev

View logs

@fatonramadani fatonramadani marked this pull request as ready for review June 25, 2024 14:45
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to e7779d0 in 1 minute and 26 seconds

More details
  • Looked at 84 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_vyzffLo6n9hHPWet


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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 5347f2c in 1 minute and 43 seconds

More details
  • Looked at 77 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_q2Idr2L9XwPKoXhR


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on d2e00e7 in 1 minute and 36 seconds

More details
  • Looked at 126 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_F6qQGkVqrEkTMehS


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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on bbb3eb8 in 1 minute and 59 seconds

More details
  • Looked at 44 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_z1IXQuQKw03q5x0s


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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on 40ac35c in 1 minute and 17 seconds

More details
  • Looked at 22 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_etx26Nf63bMAruid


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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on d3f56c3 in 1 minute and 24 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/settingsPanel/ContextVariables.svelte:57
  • Draft comment:
    The removal of unnecessary optional chaining in parentId.split('-')[0] is a good simplification since parentId is already checked for truthiness. This makes the code cleaner and more efficient.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The PR modifies the processParents function, specifically the line where parentId is parsed. The change removes unnecessary optional chaining since parentId is already checked for truthiness in the line above. This is a minor cleanup that simplifies the code and removes redundant checks.

Workflow ID: wflow_XMtbQEetfhpOViSd


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 8116e30 in 1 minute and 50 seconds

More details
  • Looked at 66 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/settingsPanel/ContextVariables.svelte:72
  • Draft comment:
    Consider moving the call to findParentsContextVariables(id) inside an onMount lifecycle function to ensure that all component properties are fully initialized before this function is executed. This can prevent potential issues with uninitialized or changing properties during component setup.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_9nip3J0r4ggW4Fwu


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@rubenfiszel rubenfiszel merged commit 73175ac into main Jun 26, 2024
3 checks passed
@rubenfiszel rubenfiszel deleted the fr/add-missing-context branch June 26, 2024 13:24
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants