-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(markdown): ensure custom style overrides precede theme colors #7441
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
base: develop
Are you sure you want to change the base?
Changes from all commits
63a00c1
d3f11f8
193cf24
40fbb6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1002,6 +1002,7 @@ | |||||
| "User_not_found_or": "User not found or incorrect password", | ||||||
| "User_sent_an_attachment": "{{user}} sent an attachment", | ||||||
| "Username": "Username", | ||||||
| "Username_invalid": "Use only letters, numbers, dots, hyphens and underscores", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Make this error message generic. Validation now follows the server-configured Suggested fix- "Username_invalid": "Use only letters, numbers, dots, hyphens and underscores",
+ "Username_invalid": "Enter a valid username",📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| "Username_is_already_in_use": "Username is already in use.", | ||||||
| "Username_not_available": "Username not available", | ||||||
| "Username_or_email": "Username or email", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -46,16 +46,25 @@ import ConfirmEmailChangeActionSheetContent from './components/ConfirmEmailChang | |||||||||||||||||||||||||||||
| const MAX_BIO_LENGTH = 260; | ||||||||||||||||||||||||||||||
| const MAX_NICKNAME_LENGTH = 120; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Mirror the server's username validation, which matches the value against the | ||||||||||||||||||||||||||||||
| // UTF8_User_Names_Validation setting anchored as a full match (`^pattern$`), | ||||||||||||||||||||||||||||||
| // falling back to the default pattern when the setting is empty or invalid. | ||||||||||||||||||||||||||||||
| // https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/app/lib/server/functions/validateUsername.ts | ||||||||||||||||||||||||||||||
| const DEFAULT_USERNAME_VALIDATION = '[0-9a-zA-Z-_.]+'; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const isValidUsername = (username: string, pattern: string): boolean => { | ||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| return new RegExp(`^(${pattern || DEFAULT_USERNAME_VALIDATION})$`).test(username); | ||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||
| // Fall back to the default pattern if the server provides an invalid regex. | ||||||||||||||||||||||||||||||
| return new RegExp(`^(${DEFAULT_USERNAME_VALIDATION})$`).test(username); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| interface IProfileViewProps { | ||||||||||||||||||||||||||||||
| navigation: NativeStackNavigationProp<ProfileStackParamList, 'ProfileView'>; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| const ProfileView = ({ navigation }: IProfileViewProps): ReactElement => { | ||||||||||||||||||||||||||||||
| const validationSchema = yup.object().shape({ | ||||||||||||||||||||||||||||||
| name: yup.string().required(I18n.t('Name_required')), | ||||||||||||||||||||||||||||||
| email: yup.string().email(I18n.t('Email_must_be_a_valid_email')).required(I18n.t('Email_required')), | ||||||||||||||||||||||||||||||
| username: yup.string().required(I18n.t('Username_required')) | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const { showActionSheet, hideActionSheet } = useActionSheet(); | ||||||||||||||||||||||||||||||
| const { colors } = useTheme(); | ||||||||||||||||||||||||||||||
| const dispatch = useDispatch(); | ||||||||||||||||||||||||||||||
|
|
@@ -67,6 +76,7 @@ const ProfileView = ({ navigation }: IProfileViewProps): ReactElement => { | |||||||||||||||||||||||||||||
| Accounts_AllowUserAvatarChange, | ||||||||||||||||||||||||||||||
| Accounts_AllowUsernameChange, | ||||||||||||||||||||||||||||||
| Accounts_CustomFields, | ||||||||||||||||||||||||||||||
| UTF8_User_Names_Validation, | ||||||||||||||||||||||||||||||
| serverVersion, | ||||||||||||||||||||||||||||||
| user | ||||||||||||||||||||||||||||||
| } = useAppSelector(state => ({ | ||||||||||||||||||||||||||||||
|
|
@@ -77,9 +87,19 @@ const ProfileView = ({ navigation }: IProfileViewProps): ReactElement => { | |||||||||||||||||||||||||||||
| Accounts_AllowUserAvatarChange: state.settings.Accounts_AllowUserAvatarChange as boolean, | ||||||||||||||||||||||||||||||
| Accounts_AllowUsernameChange: state.settings.Accounts_AllowUsernameChange as boolean, | ||||||||||||||||||||||||||||||
| Accounts_CustomFields: state.settings.Accounts_CustomFields as string, | ||||||||||||||||||||||||||||||
| UTF8_User_Names_Validation: state.settings.UTF8_User_Names_Validation as string, | ||||||||||||||||||||||||||||||
| serverVersion: state.server.version, | ||||||||||||||||||||||||||||||
| Accounts_AllowDeleteOwnAccount: state.settings.Accounts_AllowDeleteOwnAccount as boolean | ||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const validationSchema = yup.object().shape({ | ||||||||||||||||||||||||||||||
| name: yup.string().required(I18n.t('Name_required')), | ||||||||||||||||||||||||||||||
| email: yup.string().email(I18n.t('Email_must_be_a_valid_email')).required(I18n.t('Email_required')), | ||||||||||||||||||||||||||||||
| username: yup | ||||||||||||||||||||||||||||||
| .string() | ||||||||||||||||||||||||||||||
| .required(I18n.t('Username_required')) | ||||||||||||||||||||||||||||||
| .test('valid-username', I18n.t('Username_invalid'), value => isValidUsername(value ?? '', UTF8_User_Names_Validation)) | ||||||||||||||||||||||||||||||
|
Comment on lines
+98
to
+101
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Skip regex validation for unchanged or locked usernames. This test now runs on every submit, so a legacy username that no longer matches Suggested fix username: yup
.string()
.required(I18n.t('Username_required'))
- .test('valid-username', I18n.t('Username_invalid'), value => isValidUsername(value ?? '', UTF8_User_Names_Validation))
+ .test('valid-username', I18n.t('Username_invalid'), value => {
+ const nextUsername = value ?? '';
+ if (!Accounts_AllowUsernameChange || nextUsername === (user?.username ?? '')) {
+ return true;
+ }
+ return isValidUsername(nextUsername, UTF8_User_Names_Validation);
+ })📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| const isMasterDetail = useMasterDetail(); | ||||||||||||||||||||||||||||||
| const { | ||||||||||||||||||||||||||||||
| control, | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor
Assert the resolved color priority, not just presence in the style array.
The current
arrayContainingcheck only proves that a red color object appears somewhere inprops.style. It does not verify that the passedtextStyletakes visual precedence over the internal theme color. If the order were regressed (e.g.,textStyleapplied before the theme), the test would still pass while the component renders the wrong color. Refine the assertions to verify the effective resolved color or the specific position of the override object to ensure the fix is actually protected.🤖 Prompt for AI Agents