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

updated ClipboardItem and Clipboard documentation and examples using … #33262

Merged
merged 36 commits into from
May 27, 2024

Conversation

tayloregivens
Copy link
Contributor

…supports()

Updated the web API ClipboardItem and Clipboard pages to describe the supports() method. I also updated the code samples that write data to the clipboard so they use the supports method as well.

My team at Microsoft created the supports() static method and so we wanted to go ahead and document on MDN as well.

@tayloregivens tayloregivens requested a review from a team as a code owner April 24, 2024 19:10
@tayloregivens tayloregivens requested review from Elchi3 and removed request for a team April 24, 2024 19:10
@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels Apr 24, 2024
files/en-us/web/api/clipboard/write/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboard/write/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboard/write/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboard/writetext/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/clipboarditem/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
Copy link

@snianu snianu left a comment

Choose a reason for hiding this comment

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

Added some comments. PTAL, thanks!

Copy link
Contributor

github-actions bot commented Apr 25, 2024

Preview URLs

External URLs (1)

URL: /en-US/docs/Web/API/ClipboardItem/supports_static
Title: ClipboardItem: supports() static method

(comment last updated: 2024-05-27 03:35:54)

@wbamberg wbamberg requested review from wbamberg and removed request for Elchi3 April 25, 2024 15:32
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, @tayloregivens !

files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/supports/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboard/writetext/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboard/write/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboard/write/index.md Outdated Show resolved Hide resolved
@wbamberg
Copy link
Collaborator

Oh, also I missed that the directory for supports should have the _static suffix. See for example json_static or error_static in https://github.com/mdn/content/tree/main/files/en-us/web/api/response. We need to do this to support static and instance methods that have the same name, like https://developer.mozilla.org/en-US/docs/Web/API/Response/json_static and https://developer.mozilla.org/en-US/docs/Web/API/Response/json.

Copy link
Contributor

@captainbrosset captainbrosset left a comment

Choose a reason for hiding this comment

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

Drive-by approval with a couple of comments. Let's wait for Will to do a more official review.

const blob = new Blob([text], { type });
const data = [new ClipboardItem({ [type]: blob })];
await navigator.clipboard.write(data);
if (ClipboardItem.supports(type)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed offline, let's remove this change since text/plain is always supported.

Copy link
Collaborator

Choose a reason for hiding this comment

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

"image/png" is also listed as a mandatory data type: should we then change the example in the supports() page to use something from the optional list, like "image/svg+xml" ?

Copy link
Collaborator

@hamishwillee hamishwillee May 13, 2024

Choose a reason for hiding this comment

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

  1. We should list the types in supports() as per updated ClipboardItem and Clipboard documentation and examples using … #33262 (comment) - the lists are small and discrete, while the current implication of that doc is you can use any MIME type.
  2. I don't understand it yet, but the interesting optional type I'd like here is the custom type prefixed by web .

@tayloregivens (and @wbamberg) FYI, I'm showing an interest because this is being supported in FF127, and I'm part of the MDN team documenting that, - see #33564 . Great so see work already happening on this!

files/en-us/web/api/clipboard/write/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/clipboarditem/index.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thank you for the updates @tayloregivens !

I had just one more suggested change, but I think we also have an open question about whether it's appropriate to use supports() with "img/png" (#33262 (comment)). If not we should use a different MIME type for the supports() page and remove the supports() check from the examples that use "img/png" as well as the plain text ones.

files/en-us/web/api/clipboarditem/index.md Outdated Show resolved Hide resolved
@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label May 3, 2024
Copy link
Contributor

github-actions bot commented May 3, 2024

This pull request has merge conflicts that must be resolved before it can be merged.

wbamberg and others added 5 commits May 20, 2024 16:53
Since plain text is always supported no need to add support() static method to example
text always support so no need for supports()
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@hamishwillee
Copy link
Collaborator

Hi @tayloregivens
I've fixed up the example/conflicts and rebased (this is now a live example). I'll continue with necessary updates tomorrow, unless you have time to look at them.

@hamishwillee
Copy link
Collaborator

@tayloregivens I have fixed this up to address the comments. Let's see if we can get it through review!

@wbamberg Can you please look at this again. The main changes are:

  1. The ClipboardItem and supports() page examples use image/svg+xml for the mime types.
  2. Clipboard.write() still uses image/png and is a live example (merged in from main and slightly modified). I've added a note to make it clear that the supports() check is not really necessary, but highlight some cases where it could be (i.e. showing "for information").
  3. The supports() method now states the supported MIME types. Note that it omits one in the specification that no one has implemented.

@hamishwillee hamishwillee self-requested a review May 24, 2024 04:25
@tayloregivens
Copy link
Contributor Author

@tayloregivens I have fixed this up to address the comments. Let's see if we can get it through review!

@wbamberg Can you please look at this again. The main changes are:

  1. The ClipboardItem and supports() page examples use image/svg+xml for the mime types.
  2. Clipboard.write() still uses image/png and is a live example (merged in from main and slightly modified). I've added a note to make it clear that the supports() check is not really necessary, but highlight some cases where it could be (i.e. showing "for information").
  3. The supports() method now states the supported MIME types. Note that it omits one in the specification that no one has implemented.

Thanks for addressing the remaining comments and updating the 'supports()' page to use 'image/svg+xml' and making a note that supports() isn't necessary with 'image/png' on the Clipboard.write()

for 3. the supports method allows web authors to specify any arbitrary type and returns false for types that aren't supported by the browser. If you did want to add extra information there you could say that the mandatory types, html, text, and png are always supported and don't require using supports() static method.

@hamishwillee hamishwillee dismissed wbamberg’s stale review May 27, 2024 03:10

Wbamberg changes have been fixed.

wbamberg and others added 2 commits May 26, 2024 20:32
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

👍 thank you @tayloregivens and @hamishwillee for your work on this PR!

@wbamberg wbamberg merged commit 9fca458 into mdn:main May 27, 2024
8 checks passed
wbamberg added a commit to wbamberg/content that referenced this pull request May 28, 2024
* upstream/main: (55 commits)
  Replace `.` with `#` in example given selectors are `#ids` (mdn#33791)
  update info in cross browser testing strategies (mdn#33730)
  Clarify that `navigator.storage.persist()` depends on heuristics (mdn#33780)
  fix typo (mdn#33785)
  feat: improvements on Glossary/Hoisting (mdn#33787)
  CSS update: overview of shapes guide (mdn#33771)
  CSS update: Shapes from box values (mdn#33770)
  Fix issue 033506: correct droppedEntriesCount (mdn#33538)
  Revert "=== Symbol("foo")" (mdn#33782)
  docs(css): FF126 - Support for `shape()` function (mdn#33446)
  Bump lint-staged from 15.2.4 to 15.2.5 (mdn#33777)
  Bump ajv from 8.13.0 to 8.14.0 (mdn#33776)
  Add missing spaces for `subtlecrypto` (mdn#33774)
  fix: typo in `color_and_luminca` (mdn#33775)
  feat: improvments on gutters (mdn#33751)
  FF127Relnote- data: and javascript: URLS forbidden in base HREF (mdn#33738)
  update the content of SVG `<view>` element (mdn#33710)
  Clipboard.write() - log and fixes (mdn#33769)
  updated ClipboardItem and Clipboard documentation and examples using … (mdn#33262)
  Fix error in the code snippet for Symbol (mdn#33765)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants