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

Delivery Overview discard table improvements #6237

Merged
merged 25 commits into from
Mar 19, 2024
Merged

Delivery Overview discard table improvements #6237

merged 25 commits into from
Mar 19, 2024

Conversation

forstisabella
Copy link
Contributor

@forstisabella forstisabella commented Mar 15, 2024

Proposed changes

Added a table of all common delivery overview discard errors

Merge timing

When Delivery Overview (Streaming Connections) goes GA

Related issues (optional)

https://segment.atlassian.net/browse/DOC-842

@forstisabella forstisabella requested a review from a team as a code owner March 15, 2024 00:13
@forstisabella forstisabella requested review from cmastr and removed request for a team March 15, 2024 00:13
@forstisabella forstisabella added the content-update updates to content that are not new features, includes grammar fixes, added notes label Mar 15, 2024
@forstisabella forstisabella marked this pull request as draft March 15, 2024 00:20
Copy link

netlify bot commented Mar 15, 2024

Deploy Preview for segment-docs ready!

Name Link
🔨 Latest commit 6ed8287
🔍 Latest deploy log https://app.netlify.com/sites/segment-docs/deploys/65f86d61c6767b00080f23a2
😎 Deploy Preview https://deploy-preview-6237--segment-docs.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.

@forstisabella forstisabella marked this pull request as ready for review March 15, 2024 19:44
@forstisabella forstisabella changed the base branch from develop to delivery-overview-ga-one-off March 18, 2024 18:26
@cmastr
Copy link
Contributor

cmastr commented Mar 18, 2024

Will come back to review this

table = document.getElementById("settingsTable");
tr = document.getElementsByClassName("settingRow");
for (i = 0; i < tr.length; i++) {
samp = tr[i].getElementsByTagName("samp")[0];
Copy link

@ama0223 ama0223 Mar 18, 2024

Choose a reason for hiding this comment

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

if you wanna filter by both columns, you'll prob need to wrap the Discard Reason entries with a different tag but similar to how you did with <samp> then check for it in txtValue also. Or maybe assign it to a specific id like <td id=discardTitle.. if that works

Copy link

Choose a reason for hiding this comment

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

you could try it on a few entries first to make sure something like that would work then apply a search + replace all

@@ -0,0 +1,42 @@
<table>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for my own knowledge, do include tables have to be HTML instead of markdown?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, they can be markdown, most tables on the site are. but with the search and filtering functionality this table had to be formatted with HTML

Co-authored-by: Casie Oxford <coxford@twilio.com>
@forstisabella forstisabella changed the base branch from delivery-overview-ga-one-off to master March 19, 2024 14:19
@forstisabella forstisabella merged commit 31861e4 into master Mar 19, 2024
3 checks passed
@forstisabella forstisabella deleted the DOC-842 branch March 19, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content-update updates to content that are not new features, includes grammar fixes, added notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants