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

fix(frontend): accordion list header on eval / background function #5244

Merged
merged 2 commits into from
Feb 7, 2025

Conversation

alpetric
Copy link
Contributor

@alpetric alpetric commented Feb 7, 2025

Important

Fixes header display in AppAccordionList.svelte by using result[index]?.header instead of accordionInput?.value[index]?.header.

  • Behavior:
    • Fixes header display in AppAccordionList.svelte by using result[index]?.header instead of accordionInput?.value[index]?.header.
    • Ensures correct header display based on result data.

This description was created by Ellipsis for 3d75fdf. It will automatically update as commits are pushed.

@alpetric alpetric requested a review from rubenfiszel as a code owner February 7, 2025 18:12
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! Reviewed everything up to 3d75fdf in 25 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/lib/components/apps/components/display/AppAccordionList.svelte:105
  • Draft comment:
    Ensure that 'result' contains the header information. Previously, 'accordionInput' was used and provided a fallback. Verify that this change still supports proper fallback behavior when 'result[index]?.header' is undefined.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to verify that the change still supports proper fallback behavior. This is against the rule of asking the PR author to ensure the behavior is intended or to double-check things. The comment does not provide a specific suggestion or ask for a specific test to be written, which would have been acceptable.
2. frontend/src/lib/components/apps/components/display/AppAccordionList.svelte:105
  • Draft comment:
    Consider using the loop variable (e.g. {value.header}) instead of result[index]?.header. It improves clarity and avoids redundant array indexing.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None

Workflow ID: wflow_g0SgNmXdjusThvjq


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

Copy link

cloudflare-workers-and-pages bot commented Feb 7, 2025

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: acf74fc
Status: ✅  Deploy successful!
Preview URL: https://24f24daf.windmill.pages.dev
Branch Preview URL: https://alp-accordion-fix-header.windmill.pages.dev

View logs

@rubenfiszel rubenfiszel merged commit 32298e5 into main Feb 7, 2025
7 checks passed
@rubenfiszel rubenfiszel deleted the alp/accordion_fix_header branch February 7, 2025 21:55
@github-actions github-actions bot locked and limited conversation to collaborators Feb 7, 2025
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.

2 participants