Skip to content

Add material icon component generator for react - #1

Merged
alexlanz merged 34 commits into
aboutbits:masterfrom
stplasim:master
Oct 5, 2020
Merged

Add material icon component generator for react#1
alexlanz merged 34 commits into
aboutbits:masterfrom
stplasim:master

Conversation

@stplasim

@stplasim stplasim commented Oct 2, 2020

Copy link
Copy Markdown
Contributor

Add material icon component generator for react

Checklist:

  • Get a list of all Material Icons
  • Get the svg data for all icons
  • Parse Icon name to fit Conventions
  • Generate individual react components
  • Generate index page for easy import

@stplasim stplasim changed the title WIP: Add material icon component generator for react Add material icon component generator for react Oct 2, 2020
Comment thread .gitignore Outdated
@@ -0,0 +1,6 @@
# IDE Stuff
.idea

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would add more files. Maybe you can adopt the structure from the other packages:

https://github.com/aboutbits/pagination/blob/master/.gitignore

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have added the required fields

Comment thread readme.md Outdated
React Material Icons
====================

This package includes all [Material Icons](https://material.io/resources/icons/?style=baseline) as reusable react components

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would write "react" as "React", because it's the right way to write it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All names have been updated

Comment thread readme.md Outdated

## Build & Publish

To build the package, simply run the following command

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would extract the description of the build/clean command into it's own section. And keep the "Build & Publish" section equals to the other packages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Build and clean have got an own section.

Comment thread package.json Outdated
"description": "",
"main": "generator.js",
"scripts": {
"build": "node generator.js",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Attention, the build command is a reserved command that will be used when you execute "npm version patch/minor/major". That's why I would rename this command to something more explicitly named like "import".

In addition, in this file lots of required dependencies and scripts are missing. They are required for publishing the package. See: https://github.com/aboutbits/pagination/blob/master/package.json

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All necessary fields have been added. Since tsc is not required for this package, the build command executes the bootstrapping command to create the icon components

Comment thread generator.js Outdated
await generateComponent(icons.icons);
await generateIndex(icons.icons);

})(); No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would add an empty line at the end of the file.

Comment thread package.json Outdated
"postversion": "git push && git push --tags"
},
"keywords": [
"material design icons",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would split "material design" and "icons" up into two different strings to optimize the search results.

Comment thread package.json Outdated
"react"
],
"private": false,
"author": "Aboutbits",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The "B" should be written in uppercase "AboutBits".

Comment thread src/bootstrapper.js
@@ -0,0 +1,166 @@
const fs = require('fs')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would move this file outside of the src directory. I could live at the top of the directory structure. And I would include the generated files in the PR. Because, the generated code is in fact the real package. This file is just a helper file, that facilitates the generation of the files.

Comment thread readme.md Outdated
React Material Icons
====================

This package includes all [Material Icons](https://material.io/resources/icons/?style=baseline) as reusable React components

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would finish all sentenses with a dot at the end.

Comment thread readme.md
This package includes all [Material Icons](https://material.io/resources/icons/?style=baseline) as reusable React components

## Table of content
- [Usage](#usage)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add consistent empty lines before and after titles and paragraphs like in the other packages: https://raw.githubusercontent.com/aboutbits/pagination/master/readme.md

Comment thread readme.md Outdated

## Usage

In order to use this package you have to install it through npm

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would use here the same wording like in the other packages: First, ...

Comment thread readme.md Outdated
npm version major
```

To remove all generated files run the clean script

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should the user be able to clean the generated files? Or should they be automatically be deleted as first step of the generation? For example, first delete the src folder and then regenerate the files in there.

The clean task that you mention here, is to delete the dist folder. And the dist folder is just required for publishing.

@alexlanz
alexlanz merged commit 500d54b into aboutbits:master Oct 5, 2020
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.

2 participants