-
Notifications
You must be signed in to change notification settings - Fork 22.7k
Editorial review: Document FileSystemObserver #38270
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
Conversation
Preview URLs (6 pages)Flaws (6)Note! 5 documents with no flaws that don't need to be listed. 🎉 URL:
External URLs (14)URL:
URL:
URL:
URL:
URL:
(comment last updated: 2025-02-27 19:01:14) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Left a bunch of suggestions.
files/en-us/web/api/filesystemobserver/filesystemobserver/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/filesystemobserver/filesystemobserver/index.md
Outdated
Show resolved
Hide resolved
…x.md Co-authored-by: Thomas Steiner <[email protected]>
Co-authored-by: Thomas Steiner <[email protected]>
Co-authored-by: Thomas Steiner <[email protected]>
@tomayac thanks for the review! I think I've answered everything. Let me know if you feel this needs anything else, or give me an LGTM if you are happy with it. |
files/en-us/web/api/filesystemobserver/filesystemobserver/index.md
Outdated
Show resolved
Hide resolved
#### `FileSystemChangeRecord` structure | ||
|
||
`FileSystemChangeRecord` objects have the following structure: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are really two ways of dealing with dictionaries:
- document them inline as anonymous objects (so under the
records
bullet above say something like "an array of objects contain details of all the observed changes. Each object has the following properties:" - document them in separate pages, like we do for example with https://developer.mozilla.org/en-US/docs/Web/API/RequestInit.
Because this is quite a complex object it might be worth going with (2) here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've given it a go. My next commit now contains a new FileSystemChangeRecord
page, which is included in the sidebar and linked to from the appropriate places.
files/en-us/web/api/filesystemobserver/filesystemobserver/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: wbamberg <[email protected]>
Co-authored-by: wbamberg <[email protected]>
…x.md Co-authored-by: wbamberg <[email protected]>
…x.md Co-authored-by: wbamberg <[email protected]>
Co-authored-by: wbamberg <[email protected]>
Co-authored-by: wbamberg <[email protected]>
Co-authored-by: wbamberg <[email protected]>
Just confirming that https://glitch.com/edit/#!/file-system-observer?path=.prettierrc%3A1%3A0 is by me, @tomayac. It's the demo embedded in the article on chrome.dev. |
What's the license of your code, @tomayac? If we want to publish your code (at a later stage) in https://github.com/mdn/dom-examples (CCO) would that be compatible? |
All our code samples are Apache-2.0 licensed. You can publish the sample without any worries. |
Thanks @tomayac; see #38270 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of suggestions in the new FileSystemChangeRecord
page.
Co-authored-by: wbamberg <[email protected]>
Co-authored-by: wbamberg <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thank you Chris!
|
||
{{APIRef("File System API")}} | ||
|
||
The **`FileSystemChangeRecord`** dictionary of the {{domxref("File System API", "File System API", "", "nocode")}} contains details of a single change observed by a {{domxref("FileSystemObserver")}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wbamberg @chrisdavidmills Any reason for documenting this dictionary in its own page rather than inlining it, as per our typical policy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's discussed here: #38270 (comment). Arguably, because it's a large and complex object, it's easier to read the docs when it is a separate page than when it is inline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel very strongly though. I don't think this is a clear "use a dictionary" case - in particular, it is only used in that one place. But I could see the constructor page getting pretty unwieldy with all those indented lists to keep straight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK that sounds reasonable. Just making sure we are not making more undesirable stuff that we have to clean up laer.
* Document FileSystemObserver * Update files/en-us/web/api/filesystemobserver/filesystemobserver/index.md Co-authored-by: Thomas Steiner <[email protected]> * Update files/en-us/web/api/filesystemobserver/index.md Co-authored-by: Thomas Steiner <[email protected]> * Update files/en-us/web/api/file_system_api/index.md Co-authored-by: Thomas Steiner <[email protected]> * Fixes for tomayac review comments * Update files/en-us/web/api/file_system_api/index.md Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/filesystemobserver/disconnect/index.md Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/filesystemobserver/filesystemobserver/index.md Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/filesystemobserver/filesystemobserver/index.md Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/filesystemobserver/index.md Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/filesystemobserver/observe/index.md Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/filesystemobserver/observe/index.md Co-authored-by: wbamberg <[email protected]> * Add FileSystemChangeRecord page, other fixes from wbamberg review * Update files/en-us/web/api/filesystemchangerecord/index.md Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/filesystemchangerecord/index.md Co-authored-by: wbamberg <[email protected]> * Update FileSystemChangeRecord description --------- Co-authored-by: Thomas Steiner <[email protected]> Co-authored-by: wbamberg <[email protected]>
Description
Chrome 133 supports the
FileSystemObserver
interface, which allows you to observe changes to the underlying file system via a standard observer mechanism.This PR adds documentation for the new functionality, which mainly consists of new reference pages for the new interface.
Note that the feature is currently non-standard (see whatwg/fs#165), although it should be added to the spec soon. I have currently documented it as such.
Motivation
Additional details
See the relevant ChromeStatus entry for data: https://chromestatus.com/feature/4622243656630272
Relevant blog post: https://developer.chrome.com/blog/file-system-observer#start_observing_a_file_or_directory
Related issues and pull requests
Related BCD: mdn/browser-compat-data#25986