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

Allow custom room names #315

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

lebaudantoine
Copy link
Collaborator

@lebaudantoine lebaudantoine commented Jan 21, 2025

Purpose

Inspired by Jitsi's user-friendly approach, adds support for custom room names (e.g., 'team-standup', 'daily-meeting') in addition to the existing system-generated IDs (e.g., 'abc-defg-hij').

/!\ Breaking change: This introduces a permanent update to room URL patterns. Since both formats (system-generated and custom) will be shareable links, we must maintain backwards compatibility to avoid breaking existing URLs.

Proposal

Proposed UX. The UI is quite poor, will enhance it in the next iterations!

Enregistrement.de.l.ecran.2025-01-21.a.21.05.28.mov

Following numerous user requests, add support for personalized room names
alongside our existing generated IDs. This pattern, inherited from Jitsi Meet,
allows teams to create memorable, persistent meeting spaces like "daily-standup"
or "teamalpha" instead of relying only on auto-generated IDs.

Note: This change introduces a permanent update to room URL patterns.
Once custom room links are shared, we will need to maintain support for both
formats to avoid breaking existing shared URLs.
Display a "personalized" link modal, inspired by Whereby.
The UX and the UI are still cumberstome, however it's fully
functional and accessible.

It's a "okay" v0, allowing users creating their custom link.
Thus we can study this feature traction, and decide whether or
not prioritize this feature in the UX.
@manuhabitela create a well-documented modal for users trying to
join a meeting.

Update it with the last room's link format introduced.
Make the validation rules more maintainable and reusable across the codebase.
Particularly useful when these values need to be referenced in multiple
validation contexts.

The personalize room's link modal needs a step-by-step validation.
@lebaudantoine lebaudantoine force-pushed the allow-custom-room-names branch from e44e52c to 69457a0 Compare January 21, 2025 20:12
@lebaudantoine
Copy link
Collaborator Author

Discussed IRL with Arnaud: with should at least enforce 10char, and at first enables this feature with a feature flag, to only our top users, and gather early feedback.

@NathanVss
Copy link
Collaborator

Submit button should be disabled if there are errors

Capture d’écran 2025-01-27 à 17 28 21

@NathanVss
Copy link
Collaborator

Broader remark: I think using API response error names such as "error: NameExists", is much more easier to use on the frontend side instead of a message.

Using error name makes it easier to make conditions, find translations on the frontend side.

Capture d’écran 2025-01-27 à 17 32 31

Copy link
Collaborator

@NathanVss NathanVss left a comment

Choose a reason for hiding this comment

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

Nice job bud

Tbh I still have doubts about real needs for this feature as gmeet does not implement it. But anyway if users request it and it increase the adoption, lez go ;) !!

"heading": "Votre lien personnalisé",
"label": "Votre lien personnalisé",
"submit": "Créer",
"placeholder": "standupbizdev",
Copy link
Collaborator

Choose a reason for hiding this comment

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

WDYT about French placeholder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I felt bizdev was kinda french, but would you prefer, "equipecom" or smth else?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe like "Réunion Hebdo" or smth?

)
.catch((e) => {
const msg =
e.statusCode === 400
Copy link
Collaborator

Choose a reason for hiding this comment

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

Related to my comment about error names: what if there are multiple possible errors with the same http code?

Not to fix now ofc

const [serverErrors, setServerErrors] = useState<Array<string>>([])

const onSubmit = (e: FormEvent<HTMLFormElement>) => {
e.preventDefault()
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I may have overlooked something in the React Aria documentation, but this is the approach I found to prevent a HTML form from being submitted and navigating to a URL suffixed with a question mark.

Comment on lines 45 to 51
const validationErrors = []
if (roomSlug.length < 5) {
validationErrors.push(t('errors.validation.length'))
}
if (!roomSlug.match(/^[a-z0-9]+$/)) {
validationErrors.push(t('errors.validation.spaceOrSpecialCharacter'))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like how simple this approach is were most of devs wants to use libraries to validate their forms :)

const errors = [...validationErrors, ...serverErrors]

return (
<Dialog isOpen={!!isOpen} {...dialogProps} title={t('heading')}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Broader subject (out of scope): on mobile I tend to think having a full screen modal with its content being aligned on top is more UX. Mobile user are more used to panes-like navigation.

Capture d’écran 2025-01-27 à 17 38 52

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree, we have a poor mobile UX.

onAction={() => {
setIsPersonalizeModalOpen(true)
}}
data-attr="create-option-personalize"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is it used for ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's used by Posthog to scrap event data!

@lebaudantoine
Copy link
Collaborator Author

Submit button should be disabled if there are errors

In fact, it is, the form is disabled if it's invalid. But indeed the style is needed.

@lebaudantoine
Copy link
Collaborator Author

lebaudantoine commented Jan 27, 2025

Broader remark: I think using API response error names such as "error: NameExists", is much more easier to use on the frontend side instead of a message.

Using error name makes it easier to make conditions, find translations on the frontend side

I think it's the default Django error formats inherited from Joanie. Broader refactoring, to be honest.
I agree with you

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.

2 participants