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.

Sign up for GitHub

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

fix: fix: use aria attribute correctly resolves wg-translations/issue… #1162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: fix: use aria attribute correctly resolves wg-translations/issue… #1162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fadcc6a
fix: fix: use aria attribute correctly resolves wg-translations/issue…
ghassanmas Jul 16, 2024
3c5541a
squash: use labelledby and describy for advance settings
ghassanmas Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ module.exports = createConfig(
'template-curly-spacing': 'off',
'react-hooks/exhaustive-deps': 'off',
'no-restricted-exports': 'off',
// This added because 'aria-description' is not reconginzed as valid prop for reac <=17
// ref: https://github.com/facebook/react/issues/21035 because it's new.
// once this MFE upgrade to react >18, it can be removed.
'jsx-a11y/aria-props': 'off',
},
settings: {
// Import URLs should be resolved using aliases
Expand Down
14 changes: 7 additions & 7 deletions src/advanced-settings/AdvancedSettings.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,22 +137,20 @@ const AdvancedSettings = ({ intl, courseId }) => {
icon={Info}
proctoringErrorsData={proctoringErrors}
aria-hidden="true"
aria-labelledby={intl.formatMessage(messages.alertProctoringAriaLabelledby)}
aria-describedby={intl.formatMessage(messages.alertProctoringDescribedby)}
/>
)}
<TransitionReplace>
{showSuccessAlert ? (
<AlertMessage
key={intl.formatMessage(messages.alertSuccessAriaLabelledby)}
key="alert-confirmation-title"
show={showSuccessAlert}
variant="success"
icon={CheckCircle}
title={intl.formatMessage(messages.alertSuccess)}
description={intl.formatMessage(messages.alertSuccessDescriptions)}
aria-hidden="true"
aria-labelledby={intl.formatMessage(messages.alertSuccessAriaLabelledby)}
aria-describedby={intl.formatMessage(messages.alertSuccessAriaDescribedby)}
aria-label={intl.formatMessage(messages.alertSuccess)}
aria-description={intl.formatMessage(messages.alertSuccessDescriptions)}
Comment on lines -154 to +153
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still want to have aria-labelledby here, but just use the element id directly instead of using intl.formatMessage for it

Copy link
Member Author

@ghassanmas ghassanmas Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes it's one option, the thing is that two ids would need to pass from parent down to Alert componenet, I have an example here 3c5541a , if you think it's okay then I guess we do the same for all other usage?

/>
) : null}
</TransitionReplace>
Expand Down Expand Up @@ -244,8 +242,8 @@ const AdvancedSettings = ({ intl, courseId }) => {
<AlertMessage
show={saveSettingsPrompt}
aria-hidden={saveSettingsPrompt}
aria-labelledby={intl.formatMessage(messages.alertWarningAriaLabelledby)}
aria-describedby={intl.formatMessage(messages.alertWarningAriaDescribedby)}
aria-labelledby="advancedSettingsAlertWarningTitle"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
aria-labelledby="advancedSettingsAlertWarningTitle"
aria-labelledby="notification-warning-title"

Copy link
Contributor

Choose a reason for hiding this comment

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

we used have

  alertWarningAriaLabelledby: {
    id: 'course-authoring.advanced-settings.alert.warning.aria.labelledby',
    defaultMessage: 'notification-warning-title',
  },

and then we used that message in

aria-labelledby={intl.formatMessage(messages.alertWarningAriaLabelledby)}

which means this evaluates to

aria-labelledby="notification-warning-title"

aria-describedby="advancedSettingsAlertWarningDesc"
role="dialog"
actions={[
!isQueryPending && (
Expand All @@ -263,7 +261,9 @@ const AdvancedSettings = ({ intl, courseId }) => {
variant="warning"
icon={Warning}
title={intl.formatMessage(messages.alertWarning)}
titleId="advancedSettingsAlertWarningTitle"
description={intl.formatMessage(messages.alertWarningDescriptions)}
descriptoinId="advancedSettingsAlertWarningDesc"
/>
</div>
<ModalError
Expand Down
24 changes: 0 additions & 24 deletions src/advanced-settings/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,6 @@ const messages = defineMessages({
id: 'course-authoring.advanced-settings.deprecated.button.hide',
defaultMessage: 'Hide',
},
alertWarningAriaLabelledby: {
id: 'course-authoring.advanced-settings.alert.warning.aria.labelledby',
defaultMessage: 'notification-warning-title',
},
alertWarningAriaDescribedby: {
id: 'course-authoring.advanced-settings.alert.warning.aria.describedby',
defaultMessage: 'notification-warning-description',
},
alertSuccessAriaLabelledby: {
id: 'course-authoring.advanced-settings.alert.success.aria.labelledby',
defaultMessage: 'alert-confirmation-title',
},
alertSuccessAriaDescribedby: {
id: 'course-authoring.advanced-settings.alert.success.aria.describedby',
defaultMessage: 'alert-confirmation-description',
},
alertProctoringAriaLabelledby: {
id: 'course-authoring.advanced-settings.alert.proctoring.error.aria.labelledby',
defaultMessage: 'alert-danger-title',
},
alertProctoringDescribedby: {
id: 'course-authoring.advanced-settings.alert.proctoring.error.aria.describedby',
defaultMessage: 'alert-danger-description',
},
});

export default messages;
6 changes: 3 additions & 3 deletions src/course-outline/CourseOutline.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,16 +255,16 @@
/>
<TransitionReplace>
{showSuccessAlert ? (
<AlertMessage

Check failure on line 258 in src/course-outline/CourseOutline.jsx

View workflow job for this annotation

GitHub Actions / tests 

Type '{ key: string; show: true; variant: string; icon: any; title: any; description: any; "aria-hidden": string; "aria-label": any; "aria-description": any; }' is missing the following properties from type '{ [x: string]: any; title: any; titleId: any; description: any; descriptoinId: any; }': titleId, descriptoinId
key={intl.formatMessage(messages.alertSuccessAriaLabelledby)}
key="alert-confirmation-title"
show={showSuccessAlert}
variant="success"
icon={CheckCircleIcon}
title={intl.formatMessage(messages.alertSuccessTitle)}
description={intl.formatMessage(messages.alertSuccessDescription)}
aria-hidden="true"
aria-labelledby={intl.formatMessage(messages.alertSuccessAriaLabelledby)}
aria-describedby={intl.formatMessage(messages.alertSuccessAriaDescribedby)}
aria-label={intl.formatMessage(messages.alertSuccessTitle)}
aria-description={intl.formatMessage(messages.alertSuccessDescription)}
/>
) : null}
</TransitionReplace>
Expand Down
8 changes: 0 additions & 8 deletions src/course-outline/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@ const messages = defineMessages({
id: 'course-authoring.course-outline.reindex.alert.success.description',
defaultMessage: 'Course has been successfully reindexed.',
},
alertSuccessAriaLabelledby: {
id: 'course-authoring.course-outline.reindex.alert.success.aria.labelledby',
defaultMessage: 'alert-confirmation-title',
},
alertSuccessAriaDescribedby: {
id: 'course-authoring.course-outline.reindex.alert.success.aria.describedby',
defaultMessage: 'alert-confirmation-description',
},
newSectionButton: {
id: 'course-authoring.course-outline.section-list.button.new-section',
defaultMessage: 'New section',
Expand Down
5 changes: 2 additions & 3 deletions src/course-outline/page-alerts/PageAlerts.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { RequestStatus } from '../../data/constants';
import AlertMessage from '../../generic/alert-message';
import AlertProctoringError from '../../generic/AlertProctoringError';
import messages from './messages';
import advancedSettingsMessages from '../../advanced-settings/messages';
import { getPasteFileNotices } from '../data/selectors';
import { dismissError, removePasteFileNotices } from '../data/slice';
import { API_ERROR_TYPES } from '../constants';
Expand Down Expand Up @@ -195,8 +194,8 @@ const PageAlerts = ({
icon={InfoOutlineIcon}
proctoringErrorsData={proctoringErrors}
aria-hidden="true"
aria-labelledby={intl.formatMessage(advancedSettingsMessages.alertProctoringAriaLabelledby)}
aria-describedby={intl.formatMessage(advancedSettingsMessages.alertProctoringDescribedby)}
aria-label={intl.formatMessage(messages.proctoringErrorTitle)}
aria-description={intl.formatMessage(messages.proctoringErrorText)}
>
<Alert.Heading>
{intl.formatMessage(messages.proctoringErrorTitle)}
Expand Down
12 changes: 9 additions & 3 deletions src/generic/alert-message/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,27 @@ import React from 'react';
import { Alert } from '@openedx/paragon';
import PropTypes from 'prop-types';

const AlertMessage = ({ title, description, ...props }) => (
const AlertMessage = ({
title, titleId, description, descriptoinId, ...props
}) => (
<Alert {...props}>
<Alert.Heading>{title}</Alert.Heading>
<span>{description}</span>
<Alert.Heading id={titleId}>{title}</Alert.Heading>
<span id={descriptoinId}>{description}</span>
</Alert>
);

AlertMessage.propTypes = {
title: PropTypes.oneOfType([PropTypes.string, PropTypes.node]),
titleId: PropTypes.oneOfType([PropTypes.string, PropTypes.node]),
description: PropTypes.oneOfType([PropTypes.string, PropTypes.node]),
descriptoinId: PropTypes.oneOfType([PropTypes.string, PropTypes.node]),
};

AlertMessage.defaultProps = {
title: undefined,
titleId: undefined,
description: undefined,
descriptoinId: undefined,
};

export default AlertMessage;
7 changes: 1 addition & 6 deletions src/generic/create-or-rerun-course/CreateOrRerunCourseForm.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,7 @@ const CreateOrRerunCourseForm = ({
icon={InfoIcon}
title={postErrors.errMsg}
aria-hidden="true"
aria-labelledby={intl.formatMessage(
messages.alertErrorExistsAriaLabelledBy,
)}
aria-describedby={intl.formatMessage(
messages.alertErrorExistsAriaDescribedBy,
)}
aria-label={postErrors.errMsg}
/>
) : null}
</TransitionReplace>
Expand Down
8 changes: 0 additions & 8 deletions src/generic/create-or-rerun-course/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,6 @@ const messages = defineMessages({
id: 'course-authoring.create-or-rerun-course.no-space.error',
defaultMessage: 'Please do not use any spaces in this field.',
},
alertErrorExistsAriaLabelledBy: {
id: 'course-authoring.create-or-rerun-course.error.already-exists.labelledBy',
defaultMessage: 'alert-already-exists-title',
},
alertErrorExistsAriaDescribedBy: {
id: 'course-authoring.create-or-rerun-course.error.already-exists.aria.describedBy',
defaultMessage: 'alert-confirmation-description',
},
});

export default messages;
8 changes: 4 additions & 4 deletions src/generic/internet-connection-alert/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ const InternetConnectionAlert = ({
title={intl.formatMessage(messages.offlineWarningTitle)}
description={intl.formatMessage(messages.offlineWarningDescription)}
aria-hidden="true"
aria-labelledby={intl.formatMessage(
messages.offlineWarningTitleAriaLabelledBy,
aria-label={intl.formatMessage(
messages.offlineWarningTitle,
)}
aria-describedby={intl.formatMessage(
messages.offlineWarningDescriptionAriaDescribedBy,
aria-description={intl.formatMessage(
messages.offlineWarningDescription,
)}
/>
);
Expand Down
8 changes: 0 additions & 8 deletions src/generic/internet-connection-alert/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,6 @@ const messages = defineMessages({
id: 'course-authoring.generic.alert.warning.offline.description',
defaultMessage: 'This may be happening because of an error with our server or your internet connection. Try refreshing the page or making sure you are online.',
},
offlineWarningTitleAriaLabelledBy: {
id: 'course-authoring.generic.alert.warning.offline.title.aria.labelled-by',
defaultMessage: 'alert-internet-error-title',
},
offlineWarningDescriptionAriaDescribedBy: {
id: 'course-authoring.generic.alert.warning.offline.subtitle.aria.described-by',
defaultMessage: 'alert-internet-error-description',
},
});

export default messages;
8 changes: 4 additions & 4 deletions src/generic/saving-error-alert/SavingErrorAlert.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ const SavingErrorAlert = ({
errorMessage || intl.formatMessage(messages.warningDescription)
}
aria-hidden="true"
aria-labelledby={intl.formatMessage(
messages.warningTitleAriaLabelledBy,
aria-label={intl.formatMessage(
messages.warningTitle,
)}
aria-describedby={intl.formatMessage(
messages.warningDescriptionAriaDescribedBy,
aria-description={intl.formatMessage(
messages.warningDescription,
)}
/>
);
Expand Down
10 changes: 0 additions & 10 deletions src/generic/saving-error-alert/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,6 @@ const messages = defineMessages({
defaultMessage: 'This may be happening because of an error with our server or your internet connection. Try refreshing the page or making sure you are online.',
description: 'Description providing possible reasons and solutions for saving error in the studio environment',
},
warningTitleAriaLabelledBy: {
id: 'course-authoring.generic.saving-error-alert.title.aria.labelled-by',
defaultMessage: 'saving-error-alert-title',
description: 'ARIA label ID for the title of the saving error alert',
},
warningDescriptionAriaDescribedBy: {
id: 'course-authoring.generic.saving-error-alert.aria.described-by',
defaultMessage: 'saving-error-alert-description',
description: 'ARIA description ID for the saving error alert',
},
});

export default messages;
7 changes: 3 additions & 4 deletions src/grading-settings/GradingSettings.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ const GradingSettings = ({ intl, courseId }) => {
icon={CheckCircle}
title={intl.formatMessage(messages.alertSuccess)}
aria-hidden="true"
aria-labelledby={intl.formatMessage(messages.alertSuccessAriaLabelledby)}
aria-describedby={intl.formatMessage(messages.alertSuccessAriaDescribedby)}
aria-label={intl.formatMessage(messages.alertSuccess)}
/>
</div>
<div>
Expand Down Expand Up @@ -235,8 +234,8 @@ const GradingSettings = ({ intl, courseId }) => {
<AlertMessage
show={showSavePrompt}
aria-hidden={!showSavePrompt}
aria-labelledby={intl.formatMessage(messages.alertWarningAriaLabelledby)}
aria-describedby={intl.formatMessage(messages.alertWarningAriaDescribedby)}
aria-label={intl.formatMessage(messages.alertWarning)}
aria-description={intl.formatMessage(messages.alertWarningDescriptions)}
data-testid="grading-settings-save-alert"
role="dialog"
actions={[
Expand Down
16 changes: 0 additions & 16 deletions src/grading-settings/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,6 @@ const messages = defineMessages({
id: 'course-authoring.grading-settings.alert.button.cancel',
defaultMessage: 'Cancel',
},
alertWarningAriaLabelledby: {
id: 'course-authoring.grading-settings.alert.warning.aria.labelledby',
defaultMessage: 'notification-warning-title',
},
alertWarningAriaDescribedby: {
id: 'course-authoring.grading-settings.alert.warning.aria.describedby',
defaultMessage: 'notification-warning-description',
},
alertSuccessAriaLabelledby: {
id: 'course-authoring.grading-settings.alert.success.aria.labelledby',
defaultMessage: 'alert-confirmation-title',
},
alertSuccessAriaDescribedby: {
id: 'course-authoring.grading-settings.alert.success.aria.describedby',
defaultMessage: 'alert-confirmation-description',
},
creditEligibilitySectionTitle: {
id: 'course-authoring.grading-settings.credit-eligibility.title',
defaultMessage: 'Credit eligibility',
Expand Down
29 changes: 8 additions & 21 deletions src/schedule-and-details/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,8 @@ const ScheduleAndDetails = ({ intl, courseId }) => {
icon={CheckCircleIcon}
title={intl.formatMessage(messages.alertSuccess)}
aria-hidden="true"
aria-labelledby={intl.formatMessage(
messages.alertSuccessAriaLabelledby,
)}
aria-describedby={intl.formatMessage(
messages.alertSuccessAriaDescribedby,
aria-label={intl.formatMessage(
messages.alertSuccess,
)}
/>
<AlertMessage
Expand All @@ -194,11 +191,8 @@ const ScheduleAndDetails = ({ intl, courseId }) => {
icon={ErrorOutlineIcon}
title={intl.formatMessage(messages.alertLoadFail)}
aria-hidden="true"
aria-labelledby={intl.formatMessage(
messages.alertFailAriaLabelledby,
)}
aria-describedby={intl.formatMessage(
messages.alertFailAriaDescribedby,
aria-label={intl.formatMessage(
messages.alertLoadFail,
)}
/>
<AlertMessage
Expand All @@ -207,11 +201,8 @@ const ScheduleAndDetails = ({ intl, courseId }) => {
icon={ErrorOutlineIcon}
title={intl.formatMessage(messages.alertFail)}
aria-hidden="true"
aria-labelledby={intl.formatMessage(
messages.alertFailAriaLabelledby,
)}
aria-describedby={intl.formatMessage(
messages.alertFailAriaDescribedby,
aria-label={intl.formatMessage(
messages.alertFail,
)}
/>
<header>
Expand Down Expand Up @@ -351,12 +342,8 @@ const ScheduleAndDetails = ({ intl, courseId }) => {
<AlertMessage
show={showModifiedAlert}
aria-hidden={showModifiedAlert}
aria-labelledby={intl.formatMessage(
messages.alertWarningAriaLabelledby,
)}
aria-describedby={intl.formatMessage(
messages.alertWarningAriaDescribedby,
)}
aria-label={alertWhileSavingTitle}
aria-description={alertWhileSavingDescription}
role="dialog"
actions={[
!isQueryPending && (
Expand Down
Loading
Loading