Skip to content

feat(frontend) : triggers panel polishing #5825

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

Merged
merged 4 commits into from
May 28, 2025

Conversation

Guilhem-lm
Copy link
Contributor

@Guilhem-lm Guilhem-lm commented May 28, 2025

Important

Refactor trigger components to consolidate properties into a single trigger object, add DeleteTriggerButton, and improve UI/UX.

  • Refactor Trigger Properties:
    • Consolidate hasDraft, isDraftOnly, and primary into a single trigger object in SchedulePanel.svelte, GcpTriggerPanel.svelte, RoutesPanel.svelte, and other trigger-related components.
    • Update TriggerEditorToolbar.svelte to use trigger object for draft and primary checks.
  • New Component:
    • Add DeleteTriggerButton.svelte for handling trigger deletion with confirmation modal.
    • Integrate DeleteTriggerButton in TriggersTable.svelte, TriggerEditorToolbar.svelte, and other relevant components.
  • UI/UX Improvements:
    • Update warning message in CaptureSection.svelte to clarify event loss risk.
    • Remove redundant customLabel condition in TriggersWrapper.svelte and related components.
  • Miscellaneous:
    • Minor code cleanup and reorganization across multiple trigger components.

This description was created by Ellipsis for febd6ec. You can customize this summary. It will automatically update as commits are pushed.

@Guilhem-lm Guilhem-lm marked this pull request as ready for review May 28, 2025 06:32
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.

Important

Looks good to me! 👍

Reviewed everything up to febd6ec in 2 minutes and 13 seconds. Click for details.
  • Reviewed 894 lines of code in 23 files
  • Skipped 0 files when reviewing.
  • Skipped posting 17 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/src/lib/components/ScheduleEditorInner.svelte:16
  • Draft comment:
    Refactor alert: Instead of passing separate booleans (e.g. isDraftOnly), use a unified trigger object. Ensure the Trigger type includes all required properties.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. frontend/src/lib/components/triggers/TriggerEditorToolbar.svelte:37
  • Draft comment:
    Use of new 'trigger' prop: Verify the trigger object contains necessary flags (e.g. isDraft) for conditional rendering of buttons.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify the contents of the trigger object, which is against the rules as it asks for confirmation or verification. It does not provide a specific suggestion or point out a specific issue.
3. frontend/src/lib/components/triggers/TriggersTable.svelte:90
  • Draft comment:
    Confirm that DeleteTriggerButton now correctly uses the trigger prop; review triggerIconMap and TriggerLabel mappings for the unified trigger object.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. frontend/src/lib/components/triggers/TriggersWrapper.svelte:82
  • Draft comment:
    Ensure that instances where legacy boolean props were used now rely on the unified trigger object, with proper null checks if undefined.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. frontend/src/lib/components/triggers/gcp/GcpTriggerEditorInner.svelte:59
  • Draft comment:
    Review changes in GCP trigger editor: Confirm that openEdit and openNew correctly extract properties from the trigger object.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to confirm that certain functions correctly extract properties. This falls under the rule of not asking the author to confirm their intention or to ensure behavior is intended. Therefore, this comment should be removed.
6. frontend/src/lib/components/triggers/http/RouteEditorInner.svelte:27
  • Draft comment:
    Check data flow: Verify that defaultValues and trigger.draftConfig are handled consistently in openEdit/openNew using the new trigger prop.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. frontend/src/lib/components/triggers/kafka/KafkaTriggerEditorInner.svelte:22
  • Draft comment:
    Consolidation of props: Ensure Kafka trigger editor correctly extracts flags like isDraft and draftConfig from the unified trigger. Check type safety and fallbacks.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. frontend/src/lib/components/triggers/mqtt/MqttTriggerEditorInner.svelte:22
  • Draft comment:
    Unified trigger prop usage: Verify that the new pattern is propagated consistently and that loadTrigger/save functions handle errors correctly.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify that a new pattern is propagated consistently and that certain functions handle errors correctly. This falls under asking the author to double-check things and ensure behavior, which violates the rules.
9. frontend/src/lib/components/triggers/nats/NatsTriggerEditorInner.svelte:22
  • Draft comment:
    Ensure Nats trigger configuration loads and saves correctly using the unified trigger object; properly use flags like draftConfig.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. frontend/src/lib/components/triggers/postgres/PostgresTriggerEditorInner.svelte:52
  • Draft comment:
    Verify that passing the unified 'trigger' prop doesn’t break error recovery logic; ensure relation validation and required fields are computed using the trigger object state.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte:50
  • Draft comment:
    Review schedule changes: Ensure schedule-specific properties are consistently computed from the trigger or defaultValues, maintaining legacy behavior.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the author to ensure that schedule-specific properties are consistently computed, which falls under the category of asking the author to ensure behavior is intended or tested. This violates the rules.
12. frontend/src/lib/components/triggers/sqs/SqsTriggerEditorInner.svelte:20
  • Draft comment:
    SQS Trigger Editor: Verify that properties like draftConfig are properly used in openEdit/openNew and that getSaveCfg() reflects the trigger’s data.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to verify the usage of certain properties and ensure that a function reflects the trigger's data. It is not making a specific suggestion or pointing out a specific issue, but rather asking for confirmation and verification, which violates the rules.
13. frontend/src/lib/components/triggers/sqs/SqsTriggerPanel.svelte:9
  • Draft comment:
    SQS Trigger Panel updates: Confirm that the panel now passes the 'trigger' prop to the editor, ensuring correct draft/deployed status is displayed.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
14. frontend/src/lib/components/triggers/websocket/WebsocketTriggerEditorInner.svelte:29
  • Draft comment:
    WebSocket trigger editor: Ensure the new trigger prop is correctly destructured and that openEdit/openNew functions properly use trigger.draftConfig and trigger.path.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that the new trigger prop is correctly destructured and that certain functions use specific properties. This is asking the author to double-check their work, which violates the rules.
15. frontend/src/lib/components/triggers/websocket/WebsocketTriggersPanel.svelte:8
  • Draft comment:
    WebSocket Triggers Panel: Verify that selectedTrigger (passed as trigger prop) is defined and its properties (isDraft, draftConfig) are available for conditional rendering.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
16. frontend/src/lib/components/triggers/websocket/WebsocketTriggerEditorInner.svelte:52
  • Draft comment:
    Double-check that derived state (e.g. isValid, emptyString checks) now properly use trigger properties instead of separate booleans.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
17. frontend/src/lib/components/triggers/DeleteTriggerButton.svelte:21
  • Draft comment:
    Typographical note: In the confirmation modal title, there is an unnecessary space before the question mark. Consider changing "Are you sure you want to delete this draft trigger ?" to "Are you sure you want to delete this draft trigger?".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While technically correct, this is an extremely minor UI text issue. The rules state not to comment on pure UI changes and to only keep comments that require clear code changes. This feels more like a nitpick than a substantial issue that needs addressing. The spacing issue could affect user experience and professionalism of the UI. Some might argue that maintaining consistent, correct punctuation is important for product quality. While correct punctuation is good, this is too minor of an issue to warrant a code review comment. It doesn't affect functionality and falls under UI polish that we're told to ignore. Delete this comment as it's too minor and relates to UI text formatting, which we're instructed to ignore.

Workflow ID: wflow_ZvVFYqQrOGzkQuAH

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@rubenfiszel rubenfiszel merged commit af9bde3 into main May 28, 2025
14 checks passed
@rubenfiszel rubenfiszel deleted the glm/frontend/capture_refactor_v2 branch May 28, 2025 08:19
@github-actions github-actions bot locked and limited conversation to collaborators May 28, 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