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.

Sign up for GitHub

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

Jump to bottom

Complete Password Generator UI #243

Merged
merged 5 commits into from
Jun 5, 2022
Merged

Complete Password Generator UI #243

merged 5 commits into from
Jun 5, 2022

Conversation

Asterisk-z
Copy link
Contributor

Description

I designed the password generator ui and it works fine without bug but when i updated my folked repository and pull it into my mechine i get this error aside that my implementation is working great
Screenshot from 2022-05-30 03-06-39

Fixes # (issue)

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@vercel
Copy link

vercel bot commented May 30, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
react-play ✅ Ready (Inspect) Visit Preview May 31, 2022 at 10:37PM (UTC)

@atapas
Copy link
Member

atapas commented May 30, 2022

Description

I designed the password generator ui and it works fine without bug but when i updated my folked repository and pull it into my mechine i get this error aside that my implementation is working great Screenshot from 2022-05-30 03-06-39

Fixes # (issue)

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Description

I designed the password generator ui and it works fine without bug but when i updated my folked repository and pull it into my mechine i get this error aside that my implementation is working great Screenshot from 2022-05-30 03-06-39

Fixes # (issue)

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@vasantisuthar could you please help?

@atapas
Copy link
Member

atapas commented May 30, 2022

@Asterisk-z

You need to do npm install or yarn again as the incoming changes may introduce the latest dependencies.

So please stop the local server, install/update and rerun it. That should solve it.

@Asterisk-z
Copy link
Contributor Author

@Asterisk-z

You need to do npm install or yarn again as the incoming changes may introduce the latest dependencies.

So please stop the local server, install/update and rerun it. That should solve it.

Okay i will try that

@Asterisk-z
Copy link
Contributor Author

@atapas It works fine now

@atapas
Copy link
Member

atapas commented May 30, 2022

Hello @Asterisk-z

We will be moving the react-play project from the atapas account to an organization to help manage the project in a better way. The new organization URL is: https://github.com/reactplay.

The movement will take place between 12:30 - 14:00 hrs IST(7:00 am - 8:30 am GMT), 30th May 2022. Please refrain from pushing any changes between this time.

We will inform you when the movement is complete. You need to make a few minimal configuration changes after that. We will let you know.

@atapas
Copy link
Member

atapas commented May 30, 2022

Hello @Asterisk-z

We will be moving the react-play project from the atapas account to an organization to help manage the project in a better way. The new organization URL is: https://github.com/reactplay.

The movement will take place between 12:30 - 14:00 hrs IST(7:00 am - 8:30 am GMT), 30th May 2022. Please refrain from pushing any changes between this time.

We will inform you when the movement is complete. You need to make a few minimal configuration changes after that. We will let you know.

The migration is complete. You can find the project here: https://github.com/reactplay

You no need to reclone the project. You just need to set the correct upstream. Here is the link: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/configuring-a-remote-for-a-fork

@atapas atapas requested review from Angryman18 and atapas May 31, 2022 02:33
@atapas
Copy link
Member

atapas commented May 31, 2022

@Asterisk-z is it ready for review?

@Asterisk-z
Copy link
Contributor Author

@Asterisk-z is it ready for review?

Yes it is ready for review

atapas
atapas requested changes May 31, 2022
View reviewed changes
Copy link
Member

@atapas atapas left a comment

Choose a reason for hiding this comment

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

@Asterisk-z Could you please fix the responsiveness as well. The screen doesn't fit well on smaller screen devices.

src/plays/password-generator/PasswordGenerator.jsx Outdated Show resolved Hide resolved
src/plays/password-generator/PasswordGenerator.jsx Outdated Show resolved Hide resolved
@atapas
Copy link
Member

atapas commented May 31, 2022

@Angryman18

Could you please review this PR as well as the code was originated by you, it will be helpful. Thanks!

@Angryman18
Copy link
Member

@Angryman18

Could you please review this PR as well as the code was originated by you, it will be helpful. Thanks!

yes. sure.

@Angryman18
Copy link
Member

and the css looks good to me.

@Angryman18
Copy link
Member

@atapas @Asterisk-z its good now. i see an issue is not resolved yet. anyway we can omit it since the code works fine.

@atapas
Copy link
Member

atapas commented Jun 3, 2022

@atapas @Asterisk-z its good now. i see an issue is not resolved yet. anyway we can omit it since the code works fine.

@Asterisk-z are you resolving it?

@Asterisk-z
Copy link
Contributor Author

@atapas @Asterisk-z its good now. i see an issue is not resolved yet. anyway we can omit it since the code works fine.

@Asterisk-z are you resolving it?

I have fixed the responsiveness

@atapas
Copy link
Member

atapas commented Jun 5, 2022

@Angryman18 Please approve the PR if you are good. That helps me to merge it.

koustov
koustov approved these changes Jun 5, 2022
View reviewed changes
Copy link
Member

@koustov koustov left a comment

Choose a reason for hiding this comment

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

lgtm

@atapas atapas merged commit 5f66294 into reactplay:main Jun 5, 2022
@atapas
Copy link
Member

atapas commented Jun 5, 2022

@all-contributors please add @Asterisk-z for Code

@allcontributors allcontributors bot mentioned this pull request Jun 5, 2022
Merged
@allcontributors
Copy link
Contributor

@atapas

I've put up a pull request to add @Asterisk-z! 🎉

@atapas atapas linked an issue Jun 5, 2022 that may be closed by this pull request
UI Update #232
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atapas atapas atapas approved these changes

@koustov koustov koustov approved these changes

@Angryman18 Angryman18 Angryman18 approved these changes

Assignees

@Asterisk-z Asterisk-z

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UI Update
4 participants
@Asterisk-z @atapas @Angryman18 @koustov