Skip to content

Conversation

snowystinger
Copy link
Member

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

@rspbot
Copy link

rspbot commented Feb 12, 2024

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

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

Just minor comments, content looks great.

LFDanLu
LFDanLu previously approved these changes Feb 12, 2024
Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Looks fine to me just one comment but feel free to merge as is

Comment on lines 80 to 81
<PageDescription>{docs.exports.addWindowFocusTracking.description}</PageDescription>
<FunctionAPI function={docs.exports.addWindowFocusTracking} links={docs.links} />
Copy link
Member

Choose a reason for hiding this comment

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

Does <TypeLink> work here? IMO it would be nice to have the function be mentioned inline in the paragraph if so (maybe where "function" is at the moment).

@snowystinger
Copy link
Member Author

One kinda outstanding question. Is it weird to call the function with an element inside the iframe? as opposed to having people get the ownerDocument themselves? the name ends up a little odd in my opinion, but I haven't thought of anything better yet.

@rspbot
Copy link

rspbot commented Feb 12, 2024

reidbarber
reidbarber previously approved these changes Feb 12, 2024
@snowystinger
Copy link
Member Author

Looking into only exposing docs via jsdocs and not on the official docs website because we don't have our own use cases for this, meaning that it's still experimental. This should still show up in IDE's.

@rspbot
Copy link

rspbot commented Feb 12, 2024

@rspbot
Copy link

rspbot commented Feb 12, 2024

@rspbot
Copy link

rspbot commented Feb 12, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

@snowystinger snowystinger merged commit 4881d59 into main Feb 12, 2024
@snowystinger snowystinger deleted the docs-for-addwindowtofocustracking branch February 12, 2024 23:55
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.

4 participants