Skip to content

Issue resolved#61

Merged
kothariji merged 2 commits intokothariji:masterfrom
AmeyaJain-25:validation-alert-issue
Mar 15, 2021
Merged

Issue resolved#61
kothariji merged 2 commits intokothariji:masterfrom
AmeyaJain-25:validation-alert-issue

Conversation

@AmeyaJain-25
Copy link
Copy Markdown
Contributor

@AmeyaJain-25 AmeyaJain-25 commented Mar 14, 2021

This PR is realted to issue #48

There isn't a need to change the UI for alerts. It wasn't the issue related to UI, but for validation.
When I went through the code, the alert which was coming was from different thing i.e the syntaxrooms.

About the validation, when we hit create room with empty name, it leads to the syntaxRoom and the checking is done over there, whether room id or name is empty or not, which was wrong. I've added the validation on the home page modal itself. As there was no option to stop the redirection on click as we are using MUIButton, so I have used the disability property. I've disabled the buttons till the name or room id is entered or not. Room id validation is till the characters have proper 14 digit code or not. If these are fulfilled, then it allows user to click and enter or join a room. Hence validation is done properly.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 14, 2021

@AmeyaJain-25 is attempting to deploy a commit to a Personal Account owned by @kothariji on Vercel.

@kothariji first needs to authorize it.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 14, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/kothariji/syntaxmeets/9JK1gtPupye7wMJiDgEmqBJpYfuf
✅ Preview: https://syntaxmeets-git-fork-ameyajain-25-validation-alert-is-21e500.vercel.app

Copy link
Copy Markdown
Owner

@kothariji kothariji left a comment

Choose a reason for hiding this comment

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

Hi @AmeyaJain-25, thank you so much for creating the PR.

These validations are done in a nice and decent manner.

Suggested Changes

But I would suggest a slight change while validating the room-Id.
Actually our room-id follow a pattern => xxxx-yyyy-zzzz
So instead of checking the length of room-Id, it would be much better if we can validate the room-Id using regex.

here is the regex pattern - "(([A-Za-z]{4})(-)){2}[A-Za-z]{4}"
so you can match the room id with this regex, and can check if its correct or not.

You can also refer to the room-Id validation done in SyntaxRoom, it is done with regex.

Regarding Validations in Syntax Room

Actually, the validations which are there in SyntaxRoom are for those scenarios, where a user direct hops into the SyntaxRoom without joining it from home. In simple terms, a user can click on the link -https://syntaxmeets.vercel.app/tGGY-ePFw-OsaB and can join, but in this situation, we don't have a username, so to avoid such situations where user can directly hop into the room, we have to do some validations in SyntaxRoom.

Happy Sunday 🙌

@AmeyaJain-25
Copy link
Copy Markdown
Contributor Author

Yes, and that alert was showing when we enter a room without name.
So I thought to add some validation before only to avoid later checks.

Yes, I'll add the regex pattern for the room is validation and make PR again

@kothariji
Copy link
Copy Markdown
Owner

kothariji commented Mar 14, 2021

Yes, and that alert was showing when we enter a room without name.
So I thought to add some validation before only to avoid later checks.

Yes, I'll add the regex pattern for the room is validation and make PR again

yes 🔥 Great 🚀

@AmeyaJain-25
Copy link
Copy Markdown
Contributor Author

Added the Regex pattern validation

Copy link
Copy Markdown
Owner

@kothariji kothariji left a comment

Choose a reason for hiding this comment

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

Great 🚀

@kothariji kothariji added GSSOC21 JS Level2 Bug fixing, adding small features. (level 2) Priority: Medium labels Mar 15, 2021
@kothariji kothariji linked an issue Mar 15, 2021 that may be closed by this pull request
@kothariji kothariji merged commit 56345b7 into kothariji:master Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GSSOC21 JS Level2 Bug fixing, adding small features. (level 2) Priority: Medium ux-improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validation, Alert issues in Create and Join Room

2 participants