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

Add Animoto, Dailymotion block embed icons #21882

Merged
merged 4 commits into from
May 15, 2020

Conversation

enriquesanchez
Copy link
Contributor

@enriquesanchez enriquesanchez commented Apr 24, 2020

This PR adds a couple missing embed block icons (Animoto and Dailymotion), partly addressing #9124. I don't intend to add all of the missing icons here, to avoid it going stale.

Screen Shot 2020-04-24 at 17 09 38

@enriquesanchez enriquesanchez added the [Block] Embed Affects the Embed Block label Apr 24, 2020
@enriquesanchez enriquesanchez self-assigned this Apr 24, 2020
@github-actions
Copy link

github-actions bot commented Apr 24, 2020

Size Change: -1.56 kB (0%)

Total Size: 833 kB

Filename Size Change
build/annotations/index.js 3.62 kB -1 B
build/api-fetch/index.js 3.39 kB -687 B (20%) 🎉
build/autop/index.js 2.83 kB +1 B
build/block-directory/index.js 6.59 kB +358 B (5%) 🔍
build/block-directory/style-rtl.css 764 B +4 B (0%)
build/block-directory/style.css 764 B +3 B (0%)
build/block-editor/index.js 104 kB -1.81 kB (1%)
build/block-editor/style-rtl.css 10.8 kB +636 B (5%) 🔍
build/block-editor/style.css 10.8 kB +633 B (5%) 🔍
build/block-library/editor-rtl.css 7.25 kB +204 B (2%)
build/block-library/editor.css 7.25 kB +205 B (2%)
build/block-library/index.js 118 kB +5.79 kB (4%)
build/block-library/style-rtl.css 7.48 kB +341 B (4%)
build/block-library/style.css 7.49 kB +341 B (4%)
build/block-serialization-default-parser/index.js 1.88 kB +2 B (0%)
build/blocks/index.js 48.1 kB +10 B (0%)
build/components/index.js 182 kB -15.9 kB (8%)
build/components/style-rtl.css 17.1 kB +129 B (0%)
build/components/style.css 17 kB +132 B (0%)
build/compose/index.js 6.68 kB +16 B (0%)
build/core-data/index.js 11.4 kB +3 B (0%)
build/data-controls/index.js 1.29 kB +3 B (0%)
build/data/index.js 8.43 kB +2 B (0%)
build/date/index.js 5.47 kB +3 B (0%)
build/dom-ready/index.js 568 B -1 B
build/edit-navigation/index.js 5.6 kB +2.06 kB (36%) 🚨
build/edit-navigation/style-rtl.css 618 B +133 B (21%) 🚨
build/edit-navigation/style.css 617 B +132 B (21%) 🚨
build/edit-post/index.js 28.2 kB +485 B (1%)
build/edit-post/style-rtl.css 12.2 kB -72 B (0%)
build/edit-post/style.css 12.2 kB -72 B (0%)
build/edit-site/index.js 12 kB +1.05 kB (8%) 🔍
build/edit-site/style-rtl.css 5.22 kB -38 B (0%)
build/edit-site/style.css 5.22 kB -33 B (0%)
build/edit-widgets/index.js 8.47 kB +138 B (1%)
build/edit-widgets/style-rtl.css 4.69 kB -307 B (6%)
build/edit-widgets/style.css 4.69 kB -309 B (6%)
build/editor/editor-styles-rtl.css 425 B -3 B (0%)
build/editor/editor-styles.css 428 B -3 B (0%)
build/editor/index.js 44.3 kB +1 kB (2%)
build/editor/style-rtl.css 5.07 kB +1.8 kB (35%) 🚨
build/editor/style.css 5.08 kB +1.81 kB (35%) 🚨
build/element/index.js 4.65 kB -2 B (0%)
build/format-library/index.js 7.63 kB +313 B (4%)
build/i18n/index.js 3.56 kB +1 B
build/is-shallow-equal/index.js 712 B +1 B
build/keyboard-shortcuts/index.js 2.51 kB -1 B
build/keycodes/index.js 1.94 kB -1 B
build/list-reusable-blocks/index.js 3.13 kB +5 B (0%)
build/media-utils/index.js 5.29 kB +5 B (0%)
build/notices/index.js 1.79 kB -3 B (0%)
build/nux/index.js 3.4 kB +2 B (0%)
build/plugins/index.js 2.56 kB -113 B (4%)
build/primitives/index.js 1.5 kB +6 B (0%)
build/redux-routine/index.js 2.85 kB +8 B (0%)
build/server-side-render/index.js 2.68 kB +6 B (0%)
build/url/index.js 4.02 kB +1 B
build/viewport/index.js 1.84 kB +1 B
build/wordcount/index.js 1.18 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/blob/index.js 620 B 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom/index.js 3.1 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/priority-queue/index.js 789 B 0 B
build/rich-text/index.js 14.8 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/warning/index.js 1.14 kB 0 B

compressed-size-action

@enriquesanchez enriquesanchez changed the title Add Animoto icon Add Animoto, Dailymotion block embed icons Apr 24, 2020
Copy link
Contributor

@earnjam earnjam left a comment

Choose a reason for hiding this comment

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

LGTM 👍

image

@earnjam
Copy link
Contributor

earnjam commented May 14, 2020

Build fails are because of linting errors in the svgs being exported. Feels a little unnecessary, but they're getting tripped up because there are multiple attributes on the <Path> elements, so it wants them to be split out so each attribute is on it's own line.

The Dailymotion one could probably be done with foreground like the other icons, but the Animoto icon has multiple colors (looks like the first one of these with more than one color), so it has to have colors on each path.

@enriquesanchez
Copy link
Contributor Author

Thank you for the fix and the review, @earnjam! 🙏

@enriquesanchez enriquesanchez merged commit dc7ac8e into master May 15, 2020
@enriquesanchez enriquesanchez deleted the add/missing-block-embeds-icons branch May 15, 2020 00:29
@github-actions github-actions bot added this to the Gutenberg 8.2 milestone May 15, 2020
@carolinan
Copy link
Contributor

And has the license and terms of these logos been checked?

@earnjam
Copy link
Contributor

earnjam commented May 15, 2020

Good thought @carolinan. Many tech companies allow usage of their logos within apps and websites and will have brand usage guidelines to clarify their rules (Facebook, Twitter, Spotify, Vimeo, etc all have great documentation for this).

I did a quick check on the two sites included in this PR.
Animoto didn't have specific brand guidelines, but their Terms and Conditions had a section about Intellectual Property. It's only limitation on marks was this:

Animoto's trademarks and/or service marks may not be used in connection with any product or service that is not provided by Animoto, in any manner that is likely to cause confusion among customers or users of the site, tarnishes or dilutes the marks, or disparages or discredits Animoto

It's likely ok, but there are some grammar errors in that run-on sentence that make it murky.

Dailymotion has this in their API terms:

In order to use Dailymotion’s trade names, trademarks, service marks, logos, domain names (the “Dailymotion Marks”) for any purpose whatsoever, including to promote your Developer API Product(s), you must first receive written approval from Dailymotion, which may be given via email.

I'm not sure how well that would hold up where even displaying the company name for any purpose is prohibited without written authorization. By those terms we can't even have the embed block at all without asking them first. But we still want to make sure we're working with these companies and using logos according to their preferences and allowances.

Unless they have usage terms clearly spelled out online (like the ones I mentioned earlier), we probably do need to be reaching out to all platforms for which we want to bundle logos to get written confirmation. I don't imagine any would say no, but it's the safe practice.

@carolinan
Copy link
Contributor

It is also about whether the icons have been created by the team, or downloaded from a third party site.
See #21900 (comment)

@earnjam
Copy link
Contributor

earnjam commented May 16, 2020

Reading further in that thread, I think the legal perspective from Paul clears it up that it's fine to use the logos. Seems the only concern would be plucking some custom asset from a non-GPL icon library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Embed Affects the Embed Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants