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

Webhooks execution #922

Merged
merged 8 commits into from
Feb 3, 2025
Merged

Webhooks execution #922

merged 8 commits into from
Feb 3, 2025

Conversation

sans-harness
Copy link
Contributor

  • Adds the webhook-execution-list page.
Screen.Recording.2025-01-31.at.3.51.39.PM.mov

Copy link

netlify bot commented Jan 31, 2025

Deploy Preview for harness-design ready!

Name Link
🔨 Latest commit b9f72a5
🔍 Latest deploy log https://app.netlify.com/sites/harness-design/deploys/67a111ab03a64f00081fb2ed
😎 Deploy Preview https://deploy-preview-922--harness-design.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 31, 2025

Deploy Preview for harness-xd-review ready!

Name Link
🔨 Latest commit b9f72a5
🔍 Latest deploy log https://app.netlify.com/sites/harness-xd-review/deploys/67a111ab659aac00082628dc
😎 Deploy Preview https://deploy-preview-922--harness-xd-review.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@abhinavrastogi-harness
Copy link
Collaborator

Few issues observed in the video:

  1. The webhook details tab content and executions tab content are of different widths / not aligned, resulting in the page jumping horizontally when switching tabs.
  2. At the end of the video, there is a flash of the old content from the previous webhook before it renders the no-data state.

Comment on lines +11 to +12
{ id: 0, title: t('views:repos.details', 'Details'), to: 'details' },
{ id: 1, title: t('views:repos.executions', 'Executions'), to: 'executions' }
Copy link
Collaborator

Choose a reason for hiding this comment

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

navigation paths should not be hardcoded. use routes object instead

return (
<SandboxLayout.Main>
<SandboxLayout.Content maxWidth="2xl">
<h1 className="text-2xl font-medium text-foreground-1 mb-4">Order Status Update Webhook</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the title of the page hardcoded? should not be.

<SandboxLayout.Content maxWidth="2xl">
<h1 className="text-2xl font-medium text-foreground-1 mb-4">Order Status Update Webhook</h1>
<Text>
This webhook triggers every time an order status is updated, sending data to the specified endpoint for
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

]}
primaryButton={{
label: t('views:webhookData.create', 'Create webhook'),
to: `${toRepoWebhooks(repo_ref)}/create`
Copy link
Collaborator

Choose a reason for hiding this comment

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

no hardcoded routes

@sans-harness
Copy link
Contributor Author

Screen.Recording.2025-02-03.at.10.59.59.AM.mov

Fixes flciker and page jumping issue. Lets take up i18n and routes as a follow up PR?

@abhinavrastogi-harness abhinavrastogi-harness merged commit 3ed69f2 into main Feb 3, 2025
14 checks passed
@abhinavrastogi-harness abhinavrastogi-harness deleted the webhooks-execution branch February 3, 2025 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants