Add default placeholder icons #13730
Merged
+26
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cathyxz
force-pushed
the
feature/lightbox-thumbnail-placeholders
branch
from
f4f59c8
to
742bb62
Compare
cathyxz
changed the title
:crayon: Add default placeholder icons
cathyxz
changed the title
cathyxz
force-pushed
the
feature/lightbox-thumbnail-placeholders
branch
from
742bb62
to
06803db
Compare
cvializ
reviewed
Mar 1, 2018
| @@ -13,4 +13,9 @@ | |||
| * See the License for the specific language governing permissions and | |||
| * limitations under the License. | |||
| */ | |||
| export const defaultPlaceholder = 'data:image/svg+xml;charset=utf-8,<svg fill="#FFFFFF" height="24" viewBox="0 0 24 24" width="24" xmlns="http://www.w3.org/2000/svg"><path d="M6 2c-1.1 0-1.99.9-1.99 2L4 20c0 1.1.89 2 1.99 2H18c1.1 0 2-.9 2-2V8l-6-6H6zm7 7V3.5L18.5 9H13z"/><path d="M0 0h24v24H0z" fill="none"/></svg>'; | |||
| export const LIGHTBOX_THUMBNAIL_UNKNOWN = 'data:image/svg+xml;charset=utf-8,<svg width="128px" height="128px" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><defs><path d="M33,24.5 L95,24.5 C97.209139,24.5 99,26.290861 99,28.5 L99,85.452381 L80.0810811,104.5 L33,104.5 C30.790861,104.5 29,102.709139 29,100.5 L29,28.5 C29,26.290861 30.790861,24.5 33,24.5 Z" id="path-1"></path><path d="M82,83.5 L99,83.5 L99,85.5 L79.9991047,104.500895 L78,104.5 L78,87.5 C78,85.290861 79.790861,83.5 82,83.5 Z" id="path-2"></path></defs><g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd"><g id="amp_placeholders_v2" transform="translate(-613.000000, -686.000000)"></g><g id="unknown_03"><rect id="Rectangle-path" fill="#E6EFF9" x="0" y="0" width="128" height="128"></rect><g id="Rectangle-7"><use fill="#F9FCFF" fill-rule="evenodd" xlink:href="#path-1"></use><path stroke="#3C587E" stroke-width="2" d="M79.6648804,103.5 L98,85.0401517 L98,28.5 C98,26.8431458 96.6568542,25.5 95,25.5 L33,25.5 C31.3431458,25.5 30,26.8431458 30,28.5 L30,100.5 C30,102.156854 31.3431458,103.5 33,103.5 L79.6648804,103.5 Z"></path></g><g id="Rectangle"><use fill="#C1D4EE" fill-rule="evenodd" xlink:href="#path-2"></use><path stroke="#3C587E" stroke-width="2" d="M79,103.500448 L79.5850767,103.50071 L98,85.0857864 L98,84.5 L82,84.5 C80.3431458,84.5 79,85.8431458 79,87.5 L79,103.500448 Z"></path></g></g></g></svg>'; | |||
There was a problem hiding this comment.
I think these should be in CSS so they go through the SVG optimization in our build process. Use the JS to apply classnames necessary to add the background images. Right now these SVGs are not optimized for size, since I see extraneous id attributes and <g> tags. You can also manually optimize them with svgo and look for what settings we use for our automatic process.
There was a problem hiding this comment.
Oh good to know!
The only svgo options I found were these in jsify-css.js:
svgo: {
encode: true,
},
But they don't seem to mean much in svgo's cmdline tool. Am I missing something?
There was a problem hiding this comment.
Looks like the encode option is cssnano specific: https://github.com/ben-eb/cssnano/tree/master/packages/postcss-svgo
I don't think it's needed here
cvializ
approved these changes
Mar 1, 2018
| LIGHTBOX_THUMBNAIL_GIF, | ||
| LIGHTBOX_THUMBNAIL_UNKNOWN, | ||
| LIGHTBOX_THUMBNAIL_VIDEO, | ||
|
|
aghassemi
reviewed
Mar 1, 2018
|
Yup, it will be easy to switch out the icons when we finalize them. |
RanAbram
pushed a commit
to RanAbram/amphtml
that referenced
this pull request
Mar 12, 2018
* Add default placeholder icons * Manually optimize svgs with svgo * Remove extra line
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add default placeholder icons from design.