Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

Add styled components integration docs #35

Merged
merged 8 commits into from May 22, 2019
Merged

Add styled components integration docs #35

merged 8 commits into from May 22, 2019

Conversation

zaguiini
Copy link
Contributor

Fixes #32. Also updated the props.

@diego3g
Copy link
Contributor

diego3g commented May 17, 2019

Hello @zaguiini, thanks for your contribution.

We are discussing new rules for the commit message so it would be very cool if you change the commit according to our new rules.

Also, why did you changed Props to FormProps inside Form component?

@zaguiini
Copy link
Contributor Author

Ok, how can I change that since I've already committed?

Also, why did you changed Props to FormProps inside Form component?

Because it looks more specific. Props is way too generic. Every component has props, you know? But only the Form component has FormProps. 👍 It's just a way to simplify things up.

@diego3g
Copy link
Contributor

diego3g commented May 17, 2019

@zaguiini I think it's good to have naming conventions, but changing it only in the form component and not inside Input, Select, Textarea, Scope, etc, is a bit strange.

You can change the commit message using the rebase:

git rebase -i HEAD^1

Will open an editor asking what you want to do with the last commit and you change the world before the commit ID to r or reword, basically you're saying you want to rename it.

Then, save the file and you'll be asked to write a new commit message.

After that, use git push -f to force push to your branch and then commit message will be updated.

@diego3g
Copy link
Contributor

diego3g commented May 17, 2019

Just an idea, it would be AWESOME if you create a codesandbox demo using Unform and Styled Components, styling elements like Form, Input, etc.

@zaguiini
Copy link
Contributor Author

Thanks! Will do that by the weekend.

@pellizzetti
Copy link
Contributor

Ok, how can I change that since I've already committed?

Just to chime in, there's an easier way to edit your last commit message using --amend:

git commit --amend

This will open your editor with your most recent commit message, then you can fix it.

Since --amend will replace the old message, you need to force push:

git push -f

@zaguiini
Copy link
Contributor Author

Ok, how can I change that since I've already committed?

Just to chime in, there's an easier way to edit your last commit message using --amend:

git commit --amend

This will open your editor with your most recent commit message, then you can fix it.

Since --amend will replace the old message, you need to force push:

git push -f

Will this work even if I've already pushed to remote?

@pellizzetti
Copy link
Contributor

@zaguiini Yep! 😄

@zaguiini
Copy link
Contributor Author

Just an idea, it would be AWESOME if you create a codesandbox demo using Unform and Styled Components, styling elements like Form, Input, etc.

I don't this it is strictly necessary, honestly. Styled components are another topic for discussion and it takes nothing special to work with unform.

@zaguiini
Copy link
Contributor Author

No idea why it is failing. Last commit is exactly how it's said in the rules.

@diego3g
Copy link
Contributor

diego3g commented May 19, 2019

@zaguiini I think you only created new commits and didn't reword the problematic commit added styled components support, this should be replaced with the right format from our contribution guide.

Also, in your first commit docs: Add styled-components example on README, the first line have 76 characters, when only 72 is permitted.

Can you take a look at it? If you need any help please let me know.

@zaguiini
Copy link
Contributor Author

Yeah, I didn't actually reword it. Shouldn't travis work with the latest commits instead of the old ones? My latest commit is about 65 characters.

@diego3g
Copy link
Contributor

diego3g commented May 19, 2019

Travis will test all the commit messages from the PR because all of them will be integrated into master when we merge. You should take a look that Git Rebase Interactive to reorganize your commits, change messages, contents, etc.

You cant take a look here: https://thoughtbot.com/blog/git-interactive-rebase-squash-amend-rewriting-history

@zaguiini
Copy link
Contributor Author

I'm actually having trouble with it. Can we get in contact in some way?

@diego3g
Copy link
Contributor

diego3g commented May 19, 2019

@zaguiini You can talk to me in Discord (https://rocketseat.com.br/comunidade)

@diego3g
Copy link
Contributor

diego3g commented May 21, 2019

Did you get any updates @zaguiini?

@zaguiini
Copy link
Contributor Author

Sorry, can't look at it on weekdays -- college and work takes all my useful time.

@italomlp
Copy link
Contributor

@zaguiini May I fork the Unform original repo, put your code properly and open a new PR here? I'll put your name as the whole developer (like credits), and check your code too. If you prefer, give me write permissions in your repo, then I can make the needed changes. Anyway, this would be only for providing this feature ASAP.
@diego3g Is it ok to do this?

@zaguiini
Copy link
Contributor Author

@zaguiini May I fork the Unform original repo, put your code properly and open a new PR here? I'll put your name as the whole developer (like credits), and check your code too. If you prefer, give me write permissions in your repo, then I can make the needed changes. Anyway, this would be only for providing this feature ASAP.
@diego3g Is it ok to do this?

Yes!

Besides of adding examples, fixes the typings (more specific now)
and properly accepts styling via the className and style props
@italomlp
Copy link
Contributor

@zaguiini and @diego3g well, the rebase was done. I think that the only missed thing was the tests haha. Today, I don't have free time. Maybe tomorrow I can add some tests. If someone can do this, would be very welcome, since @zaguiini have no time on weekdays too.

@diego3g
Copy link
Contributor

diego3g commented May 22, 2019

@italomlp I think we don't need tests to styling at all. But i saw that Textarea was added again to your commits but it was removed. Can you merge your changes with our master to delete it?

@italomlp
Copy link
Contributor

@diego3g all fine now. Can you check it out?

@diego3g
Copy link
Contributor

diego3g commented May 22, 2019

@italomlp Great! Form.tsx is conflicting with a recent merge (#44), can you check it and adjust so we can merge this?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Move the Table of Contents to top of the README file. The
overview is still in the beginning because in a quick
search on other nice repos, I noticed that most of them
have a resume/overview on top, before the ToC. And I
think that makes sense.
@italomlp
Copy link
Contributor

@jpdemagalhaes @pellizzetti @diego3g can you check if is it ok now?

Copy link
Contributor

@pellizzetti pellizzetti left a comment

Choose a reason for hiding this comment

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

There are some unwanted changes in out last commit regarding coding style, a PR (#71) was merged to prevent this from happening again.

Fixing that I think we are ready to merge!

Copy link
Contributor

@pellizzetti pellizzetti left a comment

Choose a reason for hiding this comment

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

Sorry, I actually had to merge a PR after your merge.

Could merge master to your branch one last time? Thanks! 😅

@italomlp
Copy link
Contributor

Yes, I saw it. No problems haha. The merge was done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add styled components integration docs
5 participants