-
Notifications
You must be signed in to change notification settings - Fork 529
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
Event Webhook detail page with logs #821
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sentation from behaviour for copy to clipboard UI & behaviour
bttf
previously approved these changes
Jan 3, 2023
...nents/EventWebHooks/EventWebHookLogs/EventWebHookLogActiveItem/EventWebHookLogActiveItem.tsx
Outdated
Show resolved
Hide resolved
packages/front-end/components/EventWebHooks/EventWebHookLogs/EventWebHookLogs.tsx
Outdated
Show resolved
Hide resolved
bttf
approved these changes
Jan 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Features and Changes
SimpleTooltip component and useCopyToClipboard hook
The
<CopyToClipboard />
component had some useful UI and functionality. I didn't want all of the UI it provided, so I created 2 separate pieces of functionality to assist in the parts I did want, and de-coupled the responsibility of how stuff looks from the responsibility of the behaviour/functionality:SimpleTooltip
is the UI only for the tooltip part of the above-mentioned component, which uses the bootstrap classes. It has no behaviour and is only concerned about showing the tooltip in the desired position. It takes aposition
prop to allow devs to choose where it shows up relative to where it's placed. It does not do any kind of boundary detection so it's up to the developer to choose the position. Usually choosingtop
orbottom
will be ok, andleft
orright
works for elements that are centred in the page for mobile and desktop.useCopyToClipboard
is functionality for copying to the clipboard. The existing component was using deprecated API's so I moved to the newernavigator.clipboard
API. I retained similar functionality for checking for browser support, as well as a successful copy. As a convenience, I've added the ability to set a timeout to flip the success flag, to allow for similar implementations (hiding a tooltip after X ms). This separate hook will allow us to use copy to clipboard functionality with the current browser API's while also not being tied to any required UI in order to get that convenience.Here's a video of how tooltip position works:
tooltip.position.mov
Here's a video of the tooltip and the hook implemented:
token.copied.mov
EventWebHookDetail
I've split the detail page into 2 components, this is the 1st one. It implements the above-mentioned
SimpleTooltip
anduseCopyToClipboard
functionality to allow for copying of the webhook secret. When there is no browser support, the clipboard icon and button are hidden so the functionality is not present and the user will need to copy the text. If it is supported, the user can still copy the text manually but the button will provide a nicer UX where the clipboard icon turns into a clipboard icon with a checkmark temporarily while the copied message is visible in the temporary tooltip.Last run successful:
Last run failed:
No runs:
I've also included an example for how this UI scales when we add a lot more events. Please note, only 3 of these events are real.
Here's the copy secret functionality:
copy.secret.UX.mov
EventWebHookDetail and EventWebHookLogs
Here's an example URL to get to the page: http://localhost:3000/settings/webhooks/event/ewh-7a6d333d-165d-40ed-bb6f-1b60c02152ec
Default state:
The payload code box also expands.
Unfortunately no stories for this one because the
<Code />
component usesnext/dynamic
for lazy loading the Prism syntax highlighting and this doesn't work in Storybook.Here's a video of a webhook with both successful and failed runs:
navigating.run.logs.mov
Empty state
I'm hiding the 2-up layout when there are no runs.
Dependencies
None
Testing
Enable the
EventWebHooks
component in./packages/front-end/pages/settings/webhooks/index.tsx
and navigate to Webhooks in the sidebar. Click on event web hooks.Create an event webhook if you don't have one already.
Watch event
feature.updated
.Go to the features tab and toggle any of the environment toggles.