feat: improve recent domains section spacing and text truncation#40970
feat: improve recent domains section spacing and text truncation#40970jacquesikot merged 2 commits intoreleasefrom
Conversation
WalkthroughThis change updates the UI layout and text truncation for the recent domains section on the SignUp page, and relaxes the domain validation logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignUpPage
participant DomainUtils
User->>SignUpPage: View recent domains
SignUpPage->>DomainUtils: isValidAppsmithDomain(domain)
DomainUtils-->>SignUpPage: true/false
SignUpPage-->>User: Render recent domains with updated spacing and truncation
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
jacquesikot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
app/client/src/utils/multiOrgDomains.ts (1)
14-20: Validation now permitsappsmith.com(root) and other invalid hostnames – restore a stricter checkBy dropping the old
/^[a-z0-9-]+\.appsmith\.com$/iregex the function now treatsappsmith.com(no sub-domain) and strings like"foo bar.appsmith.com"as valid.
This will:
- Store / display root or malformed domains in the “recent domains” cookie & UI.
- Generate
https://appsmith.com/user/loginlinks that almost certainly 404.- Re-open the possibility of malformed hostnames (space, emoji, etc.) sneaking into cookies and link HREFs.
Recommend reinstating a lightweight structural check:
function isValidAppsmithDomain(domain: string): boolean { - return ( - domain.endsWith(".appsmith.com") && - !domain.startsWith("login.") && - !domain.startsWith("release.") && - !domain.startsWith("dev.") - ); + if (!domain.endsWith(".appsmith.com")) return false; + + // Reject bare apex + if (domain === "appsmith.com") return false; + + const subdomain = domain.split(".")[0]; + + // Alphanumeric & hyphen, 1–63 chars + if (!/^[a-z0-9-]{1,63}$/i.test(subdomain)) return false; + + return !["login", "release", "dev"].includes(subdomain); }This keeps the relaxed rule-set while preventing clearly invalid cases.
🧹 Nitpick comments (2)
app/client/src/pages/UserAuth/SignUp.tsx (2)
117-120: Minor: useclsx/classNamesfor conditional marginTemplate-string concatenation is OK but gets noisy fast. A tiny helper keeps things readable and avoids accidental white-space:
import clsx from "clsx"; <div className={clsx( "flex items-center justify-between p-1", index === recentDomains.length - 1 ? "mb-0" : "mb-3", )} >
127-132: Improve accessibility of truncated text
max-w-[180px] line-clamp-1neatly truncates long names, but users have no way to see the full value. Addingtitle={orgName}/title={domain}exposes the complete string on hover and screen-readers:-<span className="text-md font-semibold text-gray-700 max-w-[180px] line-clamp-1"> +<span + className="text-md font-semibold text-gray-700 max-w-[180px] line-clamp-1" + title={orgName} +>(and same for the domain line).
A tiny tweak for big UX gains.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/pages/UserAuth/SignUp.tsx(2 hunks)app/client/src/utils/multiOrgDomains.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (1)
app/client/src/utils/multiOrgDomains.ts (1)
14-20: Summary says “better accommodate dev sub-domains”, but the guard still blocksdev.*The PR description claims dev-subdomains should now be accepted, yet the function explicitly keeps
!domain.startsWith("dev.").
Please confirm which behaviour is desired and update either the code or the PR notes to avoid future confusion.Likely an incorrect or invalid review comment.
| <div | ||
| className="flex items-center justify-between p-1 mb-3" | ||
| className={`flex items-center justify-between p-1 ${ | ||
| index === recentDomains.length - 1 ? "mb-0" : "mb-3" |
There was a problem hiding this comment.
can we move this to a variable index === recentDomains.length - 1?
3 participants
📝 Summary
This PR improves the visual design and user experience of the recent domains section on the signup page with better spacing, text truncation, and simplified domain validation to account for dev and app subdomains.
🎨 Changes Made
SignUp Component (
app/client/src/pages/UserAuth/SignUp.tsx)mt-12tomt-8for better visual balancemax-w-[180px] line-clamp-1classes to organization names and domain text to:Domain Validation (
app/client/src/utils/multiOrgDomains.ts)(/^[a-z0-9-]+\.appsmith\.com$/i.test(domain))fromisValidAppsmithDomainfunction to account for dev and app subdomains.appsmith.comsuffix validation/ok-to-test tags="@tag.Sanity, @tag.Authentication"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15727210490
Commit: 5f976b0
Cypress dashboard.
Tags:
@tag.Sanity, @tag.AuthenticationSpec:
Thu, 19 Jun 2025 00:18:50 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Style
Bug Fixes