Skip to content

Conversation

dtng95
Copy link
Contributor

@dtng95 dtng95 commented Oct 2, 2025

📋 Description

  • Adding new document about creating new custom segment for Engage V16

📎 Related Issues (if applicable)

✅ Contributor Checklist

I've followed the Umbraco Documentation Style Guide and can confirm that:

  • Code blocks are correctly formatted.
  • Sentences are short and clear (preferably under 25 words).
  • Passive voice and first-person language (“we”, “I”) are avoided.
  • Relevant pages are linked.
  • All links work and point to the correct resources.
  • Screenshots or diagrams are included if useful.
  • Any code examples or instructions have been tested.
  • Typos, broken links, and broken images are fixed.

Product & Version (if relevant)

Engage V16

Deadline (if relevant)

📚 Helpful Resources

Copy link
Contributor

@nathanwoulfe nathanwoulfe left a comment

Choose a reason for hiding this comment

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

Hey @dtng95 I've left some comments/suggestions below - mainly to keep the linter happy, but also updated the manifest example.

It's probably worth adding a note too that this example depends on the Engage Backoffice NPM package, which hasn't been released yet.

@eshanrnh
Copy link
Contributor

eshanrnh commented Oct 8, 2025

Thanks for the PR, @dtng95. We will review it asap.

@eshanrnh
Copy link
Contributor

eshanrnh commented Oct 9, 2025

Hey @dtng95 👋,

Could you add the new article to the SUMMARY.md file so it appears in the navigation for review?

Copy link
Contributor

@eshanrnh eshanrnh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @dtng95 🙌 The PR looks great 💪

I have added several suggestions to remove “we,” fix vale errors and warnings, format technical terms, and align with the style guide. Let me know if anything is unclear.

Also, as mentioned above, kindly add an entry for the implement-custom-segment-parameters.md article in the SUMMARY.md file

- Update the doc based on docs team's comments
- Move the new doc to an existing doc
@dtng95 dtng95 requested a review from eshanrnh October 10, 2025 07:47
@dtng95
Copy link
Contributor Author

dtng95 commented Oct 10, 2025

Hi @eshanrnh ,
I've updated the PR, including my changes based on your comments, and move the current changes to an existing document, which already has an article in SUMMARY.md file

Copy link
Contributor

@eshanrnh eshanrnh left a comment

Choose a reason for hiding this comment

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

Thanks for incorporating the suggestions, @dtng95 🙌

The images are not showing in the Preview. Can you double-check the image paths since the article has been changed?

Copy link
Contributor

@eshanrnh eshanrnh left a comment

Choose a reason for hiding this comment

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

Attempt to fix images :)

@dtng95
Copy link
Contributor Author

dtng95 commented Oct 10, 2025

Thanks @eshanrnh for the update. Was about to push another update to fix the image, but you've already updated it. It looks good!

@dtng95 dtng95 requested a review from eshanrnh October 10, 2025 09:22
@eshanrnh eshanrnh merged commit 136fad3 into main Oct 10, 2025
43 of 45 checks passed
@eshanrnh eshanrnh deleted the v16/engage/How-to-Implement-Segment-Parameters-in-V16 branch October 15, 2025 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants