Skip to content

feat: restrict user to enter space in signup component input field#1107

Merged
iamitprakash merged 8 commits intoRealDevSquad:developfrom
azeemuddinaziz:fix/username-generation
Dec 25, 2025
Merged

feat: restrict user to enter space in signup component input field#1107
iamitprakash merged 8 commits intoRealDevSquad:developfrom
azeemuddinaziz:fix/username-generation

Conversation

@azeemuddinaziz
Copy link
Contributor

@azeemuddinaziz azeemuddinaziz commented Dec 15, 2025

Date: 15 Dec 2025

Developer Name: Azeemuddin Aziz


Issue Ticket Number:-

Description:

Previously, in the Signup Component's input field (firstname and lastname) users were able to add spaces in the input box. This PR implements to restrict users from adding any spaces in the input fields.

Is Under Feature Flag

  • Yes
  • No

Database changes

  • Yes
  • No

Breaking changes (If your feature is breaking/missing something please mention pending tickets)

  • Yes
  • No

Is Development Tested?

  • Yes
  • No

Tested in staging?

  • Yes
  • No

Add relevant Screenshot below ( e.g test coverage etc. )

screencast
restrict-space-in-input.mov
tests image

@coderabbitai
Copy link

coderabbitai bot commented Dec 15, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Input sanitization logic added to the signup component's input handler. The onChange callback now strips all whitespace from the input value before passing it to the parent, and updates the event target's value accordingly. A potential runtime issue exists with event access.

Changes

Cohort / File(s) Summary
Input sanitization
app/components/new-signup/input.js
Added whitespace stripping logic to sanitize input before onChange callback; computes sanitizedInput, updates event.target.value, and passes sanitized value to parent handler

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that event is guaranteed to be defined in scope before assigning to event.target.value
  • Confirm the whitespace-stripping approach (removing all whitespace vs. trim) is the intended behavior
  • Check if this sanitization should apply to all input fields or if there are edge cases to handle

Poem

🐰 Whitespace, be gone! With a hop and a bound,
Our inputs now sparkle, so clean and so sound!
No stray spaces linger to cause us dismay,
The sanitized signup is here to stay! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: restricting spaces in signup component input fields for firstname and lastname.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly relates to the changeset, explaining the purpose of restricting spaces in signup input fields.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f0b914 and f5c0c8f.

📒 Files selected for processing (1)
  • app/components/new-signup/input.js (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: MayankBansal12
Repo: Real-Dev-Squad/website-www PR: 1083
File: app/components/new-join-steps/new-step-one.hbs:47-56
Timestamp: 2025-11-08T13:14:05.646Z
Learning: In the Real-Dev-Squad/website-www repository onboarding form (app/components/new-join-steps/new-step-one.hbs), the fullName field is intentionally disabled as a product requirement. The name is picked from logged-in user data and users cannot change it during onboarding.

@MayankBansal12
Copy link
Member

@azeemuddinaziz Please add unit tests for the change

Copy link
Member

Choose a reason for hiding this comment

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

Should we also add sanitization during the signup form submit for first and last name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no need to add sanitization on submit because it is guaranteed that no spaces will be present at submission time (already sanitized on input change).

Copy link
Member

@MayankBansal12 MayankBansal12 Dec 16, 2025

Choose a reason for hiding this comment

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

  • what if user changes directly changes in HTML using inspect element?
  • are there going to be any changes in backend API for this? how will API handle if spaces are included in names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Adding sanitization to the from button will be redundant. The user who bypasses the onChange input sanitization by changing the value via browser dev tools will still ultimately receive an HTTP 400 Bad Request error response from the backend.

  2. No, there will be no changes in the backend API. It is already handling the input validation by using validateGenerateUsernameQuery middleware.

Copy link
Contributor

Choose a reason for hiding this comment

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

@azeemuddinaziz can you please reply to these question?

Copy link
Member

Choose a reason for hiding this comment

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

@MayankBansal12 and @Suvidh-kaushik great observation, let me drill down this

In Ember (especially with tracked state + one-way data flow), direct DOM mutation can desync the input value from the tracked property backing that input.

What can go wrong
• Cursor jumps to the end while typing
• Value reverts on re-render
• Input appears to accept spaces briefly and then snaps back
• Fails when used with @value={{this.someTrackedProp}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iamitprakash Thanks for detailed explanation.

  1. I added a keydown event that preventDefaults spaces. This completely stops the cursor jumping and desync issues since the space never hits the DOM.
  2. I still need the direct DOM manipulation for the paste edge case, where user directly pastes in a text which have spaces in it. I've updated it to only run if the input actually contains a space, so it won't interfere with normal updates.
I have added the test cases for the keydown implementation. image Screenshot 2025-12-19 at 3 30 39 PM

- I have updated the test coverage in PR description as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

@azeemuddinaziz can you please reply to these question?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Adding sanitization to the from button will be redundant. The user who bypasses the onChange input sanitization by changing the value via browser dev tools will still ultimately receive an HTTP 400 Bad Request error response from the backend.

  2. No, there will be no changes in the backend API. It is already handling the input validation by using validateGenerateUsernameQuery middleware.

- Sanitize input only when it contains a space.
lakshayman
lakshayman previously approved these changes Dec 20, 2025
Hariom01010
Hariom01010 previously approved these changes Dec 21, 2025
}

@action inputFieldChanged({ target: { value } }) {
@action inputFieldChanged(event) {
Copy link
Member

Choose a reason for hiding this comment

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

I do see issue

When you remove whitespace and calculate the new cursor position, you're using:

javascriptconst newCursorPosition = Math.min(
  nonWhitespaceBeforeCursor,
  sanitizedInput.length,
);

But nonWhitespaceBeforeCursor is the count of non-whitespace characters, not accounting for the actual position in the sanitized string. This can cause the cursor to jump incorrectly.
The Bug Scenario:
If the user types: "abc def" with cursor after 'd' (position 5):

textBeforeCursor = "abc d"
nonWhitespaceBeforeCursor = 4 (correct)
But the sanitization happens on the entire input asynchronously

The real issue is that you're setting the value and cursor position synchronously, but the value update might not be immediate, causing cursor position issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. The user cannot type "Space" in the input field.
  2. The DOM manipulation is only necessary if a user pastes text in the input directly.
  3. I still didn't completely understood the bug, because the final cursor position seems to be calculated correct.

Copy link
Member

@iamitprakash iamitprakash left a comment

Choose a reason for hiding this comment

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

comment added

@azeemuddinaziz azeemuddinaziz force-pushed the fix/username-generation branch from 19c1669 to b1615b1 Compare December 25, 2025 13:21
@iamitprakash iamitprakash merged commit 3991ab5 into RealDevSquad:develop Dec 25, 2025
3 checks passed
@azeemuddinaziz azeemuddinaziz mentioned this pull request Dec 25, 2025
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants