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

Feature: Add documents icon #579

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dustinlocke
Copy link

No description provided.

@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #579 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #579   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          34     34           
  Branches        3      3           
=====================================
  Hits           34     34

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f65819...3bb1652. Read the comment docs.

@dustinlocke dustinlocke changed the title Feature: Added documents icon Feature: Add documents icon Apr 15, 2019
Copy link

@moeenio moeenio left a comment

Choose a reason for hiding this comment

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

I think you could make them more like the file icons already existing

@colebemis
Copy link
Member

colebemis commented Apr 16, 2019

This is looking good! Just curious, can you try a version with a gap between the two pages? Similar to copy:

image

@dustinlocke
Copy link
Author

I feel you. Any of these working for anybody?
Screen Shot 2019-04-16 at 5 11 20 PM

@colebemis
Copy link
Member

@dustinlocke Thanks for trying out those options! I think I like your original version best. Let's move forward with that. I just have a couple comments:

  • Can you run npm run build (as mentioned in Need icon design guidelines #171 (comment)) to process the SVG? It'll move the stroke* attributes to the top-level svg tag and format the file.
  • Maybe this icon should be called files to be consistent with file and file-text?

@dustinlocke
Copy link
Author

@colebemis done and done!

@moeenio
Copy link

moeenio commented Jun 21, 2019

That's weird, the file looks like deleted

@mittalyashu
Copy link
Contributor

@dustinlocke, can you update your PR with the files again?

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