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

Suggestion to the PR about add React and Vue components #39

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Superhero Utils

## Installation

You can get it as npm package or from the unpkg.com.

### With npm
Expand All @@ -26,6 +25,20 @@ version for React or Vue by importing `dist/react-without-styles.js` or
`dist/vue-without-styles.js` accordingly. The framework-specific version contains
all features available in the default one plus specific for particular framework wrappers.

### Develop
Copy link
Member

Choose a reason for hiding this comment

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

I agree that this was not covered well, but have you noticed a similar section at the end of readme? I'm proposing to update it instead in a2aa925

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I saw that later after this commit

Install:
```
npm i
npm i --prefix examples/react-webpack
npm i --prefix examples/vue-webpack
```

Build scripts:
```
npm run build
npm run build:examples
```

## Usage

### Button
Expand Down
12 changes: 11 additions & 1 deletion examples/react-webpack/src/App.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
import './superhero-utils/index.css';
import { Button as SuperheroButton } from './superhero-utils/react-without-styles.js';

import './style.css';

function App() {
return (
<div>
<div className="Example-List">
<h3>superhero-utils example using react, webpack</h3>
medium:
<SuperheroButton size="medium" account="ak_... or .chain name" />

{['large', 'small', 'icon'].map(size =>
Copy link
Member

Choose a reason for hiding this comment

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

I think the initial example is fine, and we shouldn't spend any more time working on this. The purpose of building these examples was to ensure that our button is compatible with different use cases.

Copy link
Contributor Author

@shapkarin shapkarin Dec 2, 2020

Choose a reason for hiding this comment

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

I think that until we don't have any tests, it will be useful to render as much possible variances as we can, to make it easier to check if something is wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Let's combine it with something useful I'm proposing to build a code generator previewing the current state (#18). But before doing that you need to ensure that you have approval for that. cc: @mradkov

<>
{size}:
<SuperheroButton size={size} account="ak_... or .chain name" />
</>
)}
</div>
);
}
Expand Down
24 changes: 24 additions & 0 deletions examples/react-webpack/src/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
.Example-List {
width: 235px;
outline: 1px #1162fe solid;
padding: 20px;
height: 400px;
display: -ms-flexbox;
display: -webkit-flex;
display: flex;
-webkit-flex-direction: column;
-ms-flex-direction: column;
flex-direction: column;
-webkit-flex-wrap: nowrap;
-ms-flex-wrap: nowrap;
flex-wrap: nowrap;
-webkit-justify-content: space-between;
-ms-flex-pack: justify;
justify-content: space-between;
-webkit-align-content: stretch;
-ms-flex-line-pack: stretch;
align-content: stretch;
-webkit-align-items: flex-start;
-ms-flex-align: start;
align-items: flex-start;
}
34 changes: 33 additions & 1 deletion examples/vue-webpack/src/App.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
<template>
<div>
<div class="Example-List">
<h3>superhero-utils example using vue, webpack</h3>
medium:
<SuperheroButton size="medium" account="ak_... or .chain name" />
<template v-for="size in ['large', 'small', 'icon']">
{{size}}:
<SuperheroButton :size="size" account="ak_... or .chain name" :key="size" />
</template>
</div>
</template>

Expand All @@ -15,3 +20,30 @@ export default {
},
}
</script>

<style>
.Example-List {
Copy link
Member

Choose a reason for hiding this comment

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

These styles are duplicated, doesn't look good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, needs to be combined to keep all examples the same

width: 235px;
outline: 1px #1162fe solid;
padding: 20px;
height: 400px;
display: -ms-flexbox;
display: -webkit-flex;
display: flex;
-webkit-flex-direction: column;
Copy link
Member

Choose a reason for hiding this comment

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

Don't vue cli has autoprefixer enabled by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove prefixes

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that we need prefixes in sites for developers at all.

-ms-flex-direction: column;
flex-direction: column;
-webkit-flex-wrap: nowrap;
-ms-flex-wrap: nowrap;
flex-wrap: nowrap;
-webkit-justify-content: space-between;
-ms-flex-pack: justify;
justify-content: space-between;
-webkit-align-content: stretch;
-ms-flex-line-pack: stretch;
align-content: stretch;
-webkit-align-items: flex-start;
-ms-flex-align: start;
align-items: flex-start;
}
</style>