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

added quizeo play #162

Merged
merged 19 commits into from
May 27, 2022
Merged

added quizeo play #162

merged 19 commits into from
May 27, 2022

Conversation

vasantisuthar
Copy link
Contributor

I have added the Quizeo-play

Fixes #115

@vercel
Copy link

vercel bot commented May 8, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @atapas on Vercel.

@atapas first needs to authorize it.

@vercel
Copy link

vercel bot commented May 8, 2022

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

Name Status Preview Updated
react-play ✅ Ready (Inspect) Visit Preview May 27, 2022 at 4:28AM (UTC)

.gitignore Outdated Show resolved Hide resolved
src/plays/quizeo/Readme.md Outdated Show resolved Hide resolved
src/plays/quizeo/src/App.css Outdated Show resolved Hide resolved
src/plays/quizeo/src/components/header/header.styles.css Outdated Show resolved Hide resolved
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.

@vasantisuthar

For CSS Styles

Add prefix for all styles based on this comment:

"Add a play specific class name. Prefix that class name in all your style definitions. It will make sure there is no conflict with the file name.

We are planning to do this change at the framework level to take care of automatically. However, until that is in, please do this change."

And asked a few questions.

src/plays/quizeo/src/Quizeo.jsx Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@atapas
Copy link
Member

atapas commented May 10, 2022

@vasantisuthar When you take care of the review comments and questions, please resolve merge conflicts too.

After that can we do a quick call to finish the review? It is a bigger play, so just want to clarify some doubts before approving. It will be faster to merge. Need around 10-15 mins time. Let me know when based on your availability.

@vasantisuthar
Copy link
Contributor Author

@vasantisuthar When you take care of the review comments and questions, please resolve merge conflicts too.
After that can we do a quick call to finish the review? It is a bigger play, so just want to clarify some doubts before approving. It will be faster to merge. Need around 10-15 mins time. Let me know when based on your availability.

@atapas tomorrow at 3:00 pm?

@atapas atapas added work in progress The work is in progress and removed review ready labels May 18, 2022
@vasantisuthar
Copy link
Contributor Author

@atapas kindly review the changes

@atapas atapas requested a review from koustov May 23, 2022 15:14
@atapas atapas added waiting for reviewers and removed work in progress The work is in progress labels May 23, 2022
@atapas
Copy link
Member

atapas commented May 24, 2022

@vasantisuthar I picked it up for review. Please resolve the conflicts carefully.

@atapas
Copy link
Member

atapas commented May 25, 2022

@vasantisuthar Are you still making changes?

@vasantisuthar
Copy link
Contributor Author

@vasantisuthar Are you still making changes?

No, its done now.It is complete, it was just a small change to remove the logger.

package.json Outdated Show resolved Hide resolved
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.

Few more comments..

a few more to come.

src/common/routing/RouteDefs.jsx Show resolved Hide resolved
src/plays/quizeo/Readme.md Outdated Show resolved Hide resolved
src/plays/quizeo/src/Quizeo.jsx Outdated Show resolved Hide resolved
src/plays/quizeo/src/quizeo.css Outdated Show resolved Hide resolved
@atapas
Copy link
Member

atapas commented May 25, 2022

@vasantisuthar most of it looks good to me. I've noticed some impact on other plays due to conflicting styles. Could you please test it yourself using the preview URL?

To be at the safer side, better to add a unique prefix to all your styles defined in your play's CSS files.

@atapas
Copy link
Member

atapas commented May 26, 2022

@vasantisuthar most of it looks good to me. I've noticed some impact on other plays due to conflicting styles. Could you please test it yourself using the preview URL?

To be at the safer side, better to add a unique prefix to all your styles defined in your play's CSS files.

Let's plan to merge the beast this week 😁

src/plays/quizeo/src/quizeo.css Outdated Show resolved Hide resolved
@atapas atapas merged commit bcfdf7f into reactplay:main May 27, 2022
@vasantisuthar vasantisuthar deleted the vasantisuthar branch May 30, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants