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

Close Subscribe component after user subscribed #28

Merged
merged 10 commits into from
Jul 11, 2019
Merged

Close Subscribe component after user subscribed #28

merged 10 commits into from
Jul 11, 2019

Conversation

obulat
Copy link
Contributor

@obulat obulat commented Jul 7, 2019

After user clicks subscribe, the Subscribe component should be closed.

I also wrapped inputs and button in a form so that a user can also submit by pressing Enter, and made some styling changes to make it look better on smaller screens.

There seems to be a lot of changes, but most changes are due to linter applying .eslintrc.js or .editorconfig in Webstorm, I guess.

Copy link
Contributor

@erdoganbulut erdoganbulut left a comment

Choose a reason for hiding this comment

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

Did you use a formatter tool? Maybe your IDE used it for you :) . However, it is formatted in violation of our code standard.

There is no need for tab for first level rules in the style tag. It was written this way from the beginning of the project. I did not add a review item to this process because it covers the entire style tag.

We can merge after you make the changes.

Thanks for your contribution to Kodilan.

src/components/shared/Subscribe.vue Outdated Show resolved Hide resolved
src/components/shared/Subscribe.vue Outdated Show resolved Hide resolved
src/components/shared/Subscribe.vue Outdated Show resolved Hide resolved
src/components/shared/Subscribe.vue Outdated Show resolved Hide resolved
src/components/shared/Subscribe.vue Outdated Show resolved Hide resolved
@obulat
Copy link
Contributor Author

obulat commented Jul 7, 2019

Sorry about that. Yes, I used WebStorm, and it changed the formatting. I hope now it's OK.

By the way, when I run yarn serve, I get errors about linebreaks:

 Expected linebreaks to be 'LF' but found 'CRLF' (linebreak-style) at src\utils\url.js:7:3:
  5 |
  6 |   return url.indexOf('http') > -1 ? url : `http://${url}`;
> 7 | };
    |   ^
  8 |


213 errors found.
213 errors potentially fixable with the `--fix` option.

Do you know what's wrong here? I think I ran yarn serve --fix after that, and it changed formatting of the files, too...

@fatihacet
Copy link
Contributor

Thanks for contributing @obulat 👍Please add your name to Contributors list in README.md so we can give your credit for your awesome work 🎖

@erdoganbulut please review this and assign to me when it looks good to you.

@erdoganbulut
Copy link
Contributor

erdoganbulut commented Jul 8, 2019

@obulat

I think you are using Windows OS. That's why you get this error.

The '--fix' suffix applies to the 'yarn lint' command.
You can try running the 'yarn lint --fix' command.

Are all your changes done? I'll review if you add comments when finished.

Thank you again for your contribution.
We're also ready to help you resolve any bugs you've encountered.

@obulat
Copy link
Contributor Author

obulat commented Jul 8, 2019

@erdoganbulut
Yes, I'm using Windows.
I finished with changes, and you can review.

@fatihacet , @erdoganbulut
Thank you for the site! It's awesome!

@erdoganbulut
Copy link
Contributor

Congratulations and thanks. @obulat

There doesn't seem to be a problem. I added to the reviews section for @fatihacet to review.

src/components/shared/Subscribe.vue Outdated Show resolved Hide resolved
src/components/shared/Subscribe.vue Outdated Show resolved Hide resolved
@fatihacet fatihacet merged commit f282f88 into kodilan-com:master Jul 11, 2019
@fatihacet
Copy link
Contributor

LGTM ☀️ Thanks @obulat 👍

@fatihacet fatihacet mentioned this pull request Jul 11, 2019
11 tasks
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.

None yet

3 participants