Skip to content

Conversation

@weronikaolejniczak
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak commented Apr 3, 2025

Summary

On this PR, I:

Screen.Recording.2025-04-03.at.16.37.26.mov

Closes #8464

QA

Either:

  • Do a sanity check with several prop tables

or

  • test each page on the documentation site
Pages

  • Display

    • Aspect ratio
    • Avatar
    • Callout
    • Card
    • CommentList
    • DescriptionList
    • DragAndDrop
    • Health
    • Image
    • ListGroup
    • Loading
    • Progress
    • Skeleton
    • Stat
    • Text
    • Timeline
    • Title
    • Tooltip
    • Badge
    • BetaBadge
    • NotificationBadge
    • EmptyPrompt
    • Icons
    • Toast
    • Tour
  • Editors & Syntax

    • Code
    • Markdown editor
    • Markdown format
  • Forms

    • Auto refresh
    • Date picker range
    • Date picker
    • Super date picker
    • Described form group
    • Form control layouts
    • Form label
    • Form row
    • Form validation
    • Numeric > Basic
    • Numeric > Range sliders
    • Other > Color picker
    • Other > File picker
    • Search & Filter > Expression
    • Search & Filter > Filter group
    • Search & Filter > Search bar
    • Search & Filter > Search field
    • Selection > Combo Box
    • Selection > Select
    • Selection > Selectable
    • Selection > Super select
    • Selection > Checkboxes and radios
    • Selection > Switch
    • Text > Basic
    • Text > Inline edit
    • Text > Password
  • Layout

    • Accordion
    • Bottom bar
    • Header
    • Horizontal rule
    • Page header
    • Popover
    • Resizable container
    • Spacer
    • Flex > Flex grid
    • Flex > Flex group
    • Flex > Flex item
    • Flyout > Flyout push
    • Flyout > Flyout resizable
    • Flyout
    • Confirm modal
    • Modal
    • Page components
    • Panel
    • Split panel
  • Navigation

    • Breadcrumbs
    • Collapsible nav
    • Context menu
    • Facet
    • Key pad menu
    • Link
    • Side nav
    • Tree view
    • Button basic
    • Button empty
    • Button group
    • Button icon
    • Pagination
    • Horizontal steps
    • Steps
    • Tabs
  • Tabular content

    • Data grid
    • Data grid > Cells and popover
    • Data grid > Container constraints
    • Data grid > Schema and columns
    • Data grid > Style and display
    • Data grid > Toolbar
    • Data grid > In memory
    • Data grid > Ref
    • Data grid > Custom body rendering
    • Tables > Basic tables
    • Tables > Custom tables
    • Tables > In-memory tables
    • Tables > Custom cell rendering
    • Tables > Sorting and filtering
    • Tables > Pagination
    • Tables > Row selection
    • Tables > Table actions
    • Tables > Fixed header/columns
    • Tables > Virtual scrolling
    • Tables > Drag and drop rows
    • Tables > Expandable rows
  • Templates

    • Sitewide search
    • Page template
    • Breakpoints > Utilities
  • Utilities

    • Auto sizer
    • Beacon
    • Copy
    • Delay
    • Error boundary
    • Highlight and mark
    • HTML id generator
    • i18n
    • Inner text
    • Mutation observer
    • Outside click detector
    • Overlay mask
    • Portal
    • Pretty duration
    • Provider
    • Resize observer
    • Text diff
    • Text truncation
    • Accessibility
    • Color palettes
    • Focus trap
    • Window events

@weronikaolejniczak weronikaolejniczak added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Apr 3, 2025
@weronikaolejniczak weronikaolejniczak changed the title Feat/eui plus prop table links [EUI+] Prop table links in the description are broken Apr 4, 2025
@weronikaolejniczak weronikaolejniczak force-pushed the feat/eui-plus-prop-table-links branch 2 times, most recently from 3ea00c7 to 32177e2 Compare April 4, 2025 10:12
@weronikaolejniczak weronikaolejniczak deleted the feat/eui-plus-prop-table-links branch April 16, 2025 06:46
@weronikaolejniczak weronikaolejniczak restored the feat/eui-plus-prop-table-links branch April 16, 2025 06:48
@weronikaolejniczak weronikaolejniczak force-pushed the feat/eui-plus-prop-table-links branch from 266c1dc to f2c043a Compare April 25, 2025 10:52
@weronikaolejniczak weronikaolejniczak marked this pull request as ready for review April 29, 2025 12:07
@weronikaolejniczak weronikaolejniczak requested a review from a team as a code owner April 29, 2025 12:07
@weronikaolejniczak weronikaolejniczak changed the title [EUI+] Prop table links in the description are broken feat(docusaurus-theme): redirect to GH using types definition Apr 29, 2025
@acstll acstll self-requested a review April 30, 2025 07:50
@weronikaolejniczak
Copy link
Contributor Author

weronikaolejniczak commented Apr 30, 2025

@acstll you will notice that sometimes there's other JSDoc annotations that do not work (@see aria-labelledby(corrected)), if you have an idea how to tackle this I'm open to doing it on this PR. Maybe we should remove all of them when parsing the description?

Sometimes there are also @see {@link something} which for some reason doesn't parse correctly, those links are skipped. I believe the approach could be improving the regexp OR removing @see before @link in the comments.

Let me know what you think! Looking forward to your review 🙏🏻

If we could move bigger updates to separate tasks that'd be cool as well, so that I am able to wrap this up before EAH.

@weronikaolejniczak weronikaolejniczak force-pushed the feat/eui-plus-prop-table-links branch from 2e6ebb7 to 255a107 Compare May 5, 2025 09:12
Copy link
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

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

I tested this thoroughly and it's a really nice improvement, LGTM 🟢

Some notes:

  • I think it's OK to replace @see with plain See when it's preceding a {@link …} (JSDoc docs give that impression)
  • as discussed offline, external links and other link-like items like @see aria-labelledby can be handled later on, and this PR can focus on internal {@link (\w+)} which works pretty good imho
  • probably most external links and similar come from extending types not-in-our-codebase (also discussed offline)

only for reference, these are examples of external and link-like items:

ref in https://eui.elastic.co/pr_8543/docs/components/display/badge/#EuiBadgeGroup

@see {@link https://react.dev/learn/referencing-values-with-refs#refs-and-the-dom React Docs}

aria-label in many places

@see aria-labelledby.

language in https://eui.elastic.co/pr_8543/docs/components/display/code/#EuiCode

@see https://prismjs.com/#supported-languages for options

@weronikaolejniczak weronikaolejniczak force-pushed the feat/eui-plus-prop-table-links branch from 9561655 to 10535fb Compare May 5, 2025 11:40
@weronikaolejniczak weronikaolejniczak self-assigned this May 5, 2025
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @weronikaolejniczak

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @weronikaolejniczak

Copy link
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

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

[bonus comment] while running the typedoc command, we get 415 warnings… I'm wondering how many of those are things that would be beneficial to address, and how many false positives or things we can safely ignore (speaking of which, I'm not familiar with typedoc) — do you think this is something we could address in this PR? later on?

@weronikaolejniczak
Copy link
Contributor Author

weronikaolejniczak commented May 5, 2025

@acstll thanks for reminding me about warnings. I think it's safe to proceed with what we already have but could be beneficial to look through the list. The majority of them are about not exporting an interface or unexpected JSDoc annotations. Potentially something that might not end up in the JSON output and not be linked in the description to GitHub BUT would have to be checked one by one. We can totally do that work on another ticket while already incorporating a lot of value to the docs site with this PR 😄 what do you think?

Side note, these warnings can be silenced.

@weronikaolejniczak weronikaolejniczak merged commit 3c16315 into elastic:main May 6, 2025
5 checks passed
@acstll
Copy link
Contributor

acstll commented May 6, 2025

We can totally do that work on another ticket while already incorporating a lot of value to the docs site with this PR 😄 what do you think?

I agree! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Docs] Prop table links in the description are broken

3 participants