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

New terminology: ClipboardItem "entry" as data type → value. #127

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lgarron
Copy link
Contributor

@lgarron lgarron commented Jun 25, 2020

Right now, the spec refers to two things as "items":

  • A list of ClipboardItems.
  • The individual data values inside a single ClipboardItem.

This is rather confusing, so this PR proposes to rename the latter to
"entries", where an entry conceptually corresponds to a data type (e.g.
"text/string") that maps to a value (e.g. a Blob). This:

  • Matches the JS naming convention (think of Object.entries()).
  • Does not change any spec semantics, only the names of some identifiers.

In addition, this PR also renames:

  • ClipboardItemDataTypeClipboardItemValue, and
  • ClipboardItemDataClipboardItemValuePromise.

This matches the "entry is a type mapping to a value" convention,
and makes it clear what is a Promise. The previous names may have caused
a semantic typo (#126).

Closes #????

The following tasks have been completed:

  • Confirmed there are no ReSpec/BikeShed errors or warnings.
  • Modified Web platform tests (link to pull request)

Implementation commitment:


Preview | Diff

@lgarron lgarron changed the title New terminology: ClipboardItem entry as data type → value. New terminology: ClipboardItem "entry" as data type → value. Jun 25, 2020
Right now, the spec refers to two things as "items":

- A list of `ClipboardItem`s.
- The individual data values inside a single `ClipboardItem`.

This is rather confusing, so this PR proposes to rename the latter to
"entries", where an entry conceptually corresponds to a data type (e.g.
`"text/string"`) that maps to a value (e.g. a Blob). This:

- Matches the JS naming convention (think of `Object.entries`).
- Does not change any spec semantics, only the names of some identifiers.

In addition, this PR also renames:

- `ClipboardItemDataType` → `ClipboardItemValue`, and
- `ClipboardItemData` → `ClipboardItemValuePromise`.

This matches the "entry is a type type mapping to a value" convention,
and makes it clear what is a Promise. The previous names may have caused
a semantic typo (w3c#126).

[Exposed=Window] interface ClipboardItem {
constructor(record<DOMString, ClipboardItemData> items,
constructor(record<DOMString, ClipboardEntryValue> entries,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change assumes that #126 is correct (e.g. matching Chrome's current behaviour). If this should accept Promises, then either:

  • the record values should be ClipboardEntryValuePromise and Chrome's implementation should be changed, or
  • the record values should be allowed to be either ClipboardEntryValue or ClipboardEntryValuePromise.

@lgarron
Copy link
Contributor Author

lgarron commented Jun 25, 2020

FYI: my W3C affiliation is set to GitHub, Inc., but this my participation in this repo should be considered under my Chromium affiliation. I don't know if there's a way to tell the CI this.

@siusin
Copy link
Contributor

siusin commented Dec 22, 2020

@lgarron the IPR issue has been fixed by @travisleithead and our Systeam. It will no longer be a blocker for future contributions.

Base automatically changed from master to main February 3, 2021 04:30
@mbrodesser
Copy link

@lgarron: are you still working on this PR? The change itself is desirable, it just requires slight adaptation to match the current base.

@lgarron
Copy link
Contributor Author

lgarron commented Jun 2, 2021

@lgarron: are you still working on this PR? The change itself is desirable, it just requires slight adaptation to match the current base.

Yeah, I'd be glad to do that! Please give me a few days! 🙏

@mbrodesser
Copy link

@lgarron: just to be sure, do you still intend adapting this patch soon? It would be valuable.

@mbrodesser
Copy link

@lgarron: can you please give an update if you still intend to adapt this patch?

Copy link
Member

@travisleithead travisleithead left a comment

Choose a reason for hiding this comment

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

These IDL changes don't impact any behavior, and they make sense to me.

@mbrodesser mbrodesser mentioned this pull request Oct 7, 2021
4 tasks
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