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

Issue 10255 improve coach tabs accessibility #11501

Merged
merged 5 commits into from
Nov 30, 2023
Merged

Issue 10255 improve coach tabs accessibility #11501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bb89506
Add new constants for Learners tab
muditchoudhary Nov 4, 2023
855e993
Use KDS tabs in Coach Repors Learners tab
muditchoudhary Nov 4, 2023
9d4f675
Fix: Focus on active tab when navigating with a keyboard
muditchoudhary Nov 19, 2023
3c11757
Fix foucs ring disappears from tabs after tab swtiched
muditchoudhary Nov 22, 2023
84fc429
Fix linting
muditchoudhary Nov 25, 2023
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
6 changes: 6 additions & 0 deletions kolibri/plugins/coach/assets/src/constants/tabsConstants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,9 @@ export const ReportsGroupTabs = {
MEMBERS: 'tabMembers',
ACTIVITY: 'tabActivity',
};

export const LEARNERS_TABS_ID = 'reportLearners';
Copy link
Member

Choose a reason for hiding this comment

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

Just a nitpick, no change required. May be nice to follow the naming convention of other constants and use 'coachReportsLearners' as ID value.

export const LearnersTabs = {
REPORTS: 'tabReports',
ACTIVITY: 'tabActivity',
};
24 changes: 17 additions & 7 deletions kolibri/plugins/coach/assets/src/views/reports/ReportsLearnerActivityPage.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@

<KPageContainer>

<ReportsLearnerHeader :enablePrint="true" />

<ActivityList
embeddedPageName="ReportsLearnerActivityPage"
:noActivityString="coachString('activityListEmptyState')"
/>

<ReportsLearnerHeader :enablePrint="true" :activeTabId="LearnersTabs.ACTIVITY" />
<KTabsPanel
:tabsId="LEARNERS_TABS_ID"
:activeTabId="LearnersTabs.ACTIVITY"
>
<ActivityList
embeddedPageName="ReportsLearnerActivityPage"
:noActivityString="coachString('activityListEmptyState')"
/>
</KTabsPanel>
</KPageContainer>
</CoachAppBarPage>

Expand All @@ -26,6 +29,7 @@
import commonCoach from '../common';
import CoachAppBarPage from '../CoachAppBarPage';
import ActivityList from '../common/notifications/ActivityList';
import { LEARNERS_TABS_ID, LearnersTabs } from '../../constants/tabsConstants';
import ReportsLearnerHeader from './ReportsLearnerHeader';

export default {
Expand All @@ -36,6 +40,12 @@
ReportsLearnerHeader,
},
mixins: [commonCoach],
data() {
return {
LEARNERS_TABS_ID,
LearnersTabs,
};
},
};

</script>
Expand Down
62 changes: 55 additions & 7 deletions kolibri/plugins/coach/assets/src/views/reports/ReportsLearnerHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@
</HeaderTableRow>
</HeaderTable>
<HeaderTabs :enablePrint="enablePrint">
<HeaderTab
:text="coachString('reportsLabel')"
:to="classRoute('ReportsLearnerReportPage', {})"
/>
<HeaderTab
:text="coachString('activityLabel')"
:to="classRoute('ReportsLearnerActivityPage', {})"
<KTabsList
ref="tabList"
:tabsId="LEARNERS_TABS_ID"
:ariaLabel="$tr('reportLearners')"
:activeTabId="activeTabId"
:tabs="tabs"
@click="() => saveTabsClick(LEARNERS_TABS_ID)"
/>
</HeaderTabs>
</div>
Expand All @@ -71,16 +71,34 @@

import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';
import commonCoach from '../common';
import { LEARNERS_TABS_ID, LearnersTabs } from '../../constants/tabsConstants';
import { useCoachTabs } from '../../composables/useCoachTabs';

export default {
name: 'ReportsLearnerHeader',
mixins: [commonCoach, commonCoreStrings],
setup() {
const { saveTabsClick, wereTabsClickedRecently } = useCoachTabs();
return {
saveTabsClick,
wereTabsClickedRecently,
};
},
props: {
enablePrint: {
type: Boolean,
required: false,
default: false,
},
activeTabId: {
type: String,
required: true,
},
},
data() {
return {
LEARNERS_TABS_ID,
};
},
computed: {
learner() {
Expand Down Expand Up @@ -115,13 +133,43 @@
);
return statuses.length;
},
tabs() {
return [
{
id: LearnersTabs.REPORTS,
label: this.coachString('reportsLabel'),
to: this.classRoute('ReportsLearnerReportPage', {}),
},
{
id: LearnersTabs.ACTIVITY,
label: this.coachString('activityLabel'),
to: this.classRoute('ReportsLearnerActivityPage', {}),
},
];
},
},
mounted() {
// focus the active tab but only when it's likely
// that this header was re-mounted as a result
// of navigation after clicking a tab (focus shouldn't
// be manipulated programatically in other cases, e.g.
// when visiting the Plan page for the first time)
Copy link
Member

Choose a reason for hiding this comment

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

Minor comment: "when visiting the Plan page" This is not the Plan page. I think that's result of copy-pasting, so perhaps we could just say "when visiting the page" to make future copies work without the need to adjust it

if (this.wereTabsClickedRecently(this.LEARNERS_TABS_ID)) {
this.$nextTick(() => {
this.$refs.tabList.focusActiveTab();
});
}
},
$trs: {
back: {
message: 'All learners',
context:
"Link that takes user back to the list of learners on the 'Reports' tab, from the individual learner's information page.",
},
reportLearners: {
message: 'Report learners',
context: 'Labels the Reports > Learners tab for screen reander users',
Copy link
Member

Choose a reason for hiding this comment

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

A little typo: "...screen reader"

},
},
};

Expand Down
163 changes: 87 additions & 76 deletions kolibri/plugins/coach/assets/src/views/reports/ReportsLearnerReportPage.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,84 +8,88 @@

<KPageContainer>

<ReportsLearnerHeader />

<ReportsControls :disableExport="true" />

<KGrid>
<KGridItem :layout12="{ span: $isPrint ? 12 : 6 }">
<h2>{{ coachString('lessonsAssignedLabel') }}</h2>
<CoreTable :emptyMessage="coachString('lessonListEmptyState')">
<template #headers>
<th>{{ coachString('titleLabel') }}</th>
<th>{{ coreString('progressLabel') }}</th>
</template>
<template #tbody>
<transition-group
tag="tbody"
name="list"
>
<tr
v-for="tableRow in lessonsTable"
:key="tableRow.id"
<ReportsLearnerHeader :activeTabId="LearnersTabs.REPORTS" />
<KTabsPanel
:tabsId="LEARNERS_TABS_ID"
:activeTabId="LearnersTabs.REPORTS"
>
<ReportsControls :disableExport="true" />

<KGrid>
<KGridItem :layout12="{ span: $isPrint ? 12 : 6 }">
<h2>{{ coachString('lessonsAssignedLabel') }}</h2>
<CoreTable :emptyMessage="coachString('lessonListEmptyState')">
<template #headers>
<th>{{ coachString('titleLabel') }}</th>
<th>{{ coreString('progressLabel') }}</th>
</template>
<template #tbody>
<transition-group
tag="tbody"
name="list"
>
<td>
<KRouterLink
:to="classRoute('ReportsLearnerReportLessonPage', {
lessonId: tableRow.id
})"
:text="tableRow.title"
icon="lesson"
/>
</td>
<td>
<StatusSimple :status="tableRow.status" />
</td>
</tr>
</transition-group>
</template>
</CoreTable>
</KGridItem>
<KGridItem :layout12="{ span: $isPrint ? 12 : 6 }">
<h2>{{ coachString('quizzesAssignedLabel') }}</h2>
<CoreTable
:class="{ print: $isPrint }"
:emptyMessage="coachString('quizListEmptyState')"
>
<template #headers>
<th>{{ coachString('titleLabel') }}</th>
<th>{{ coreString('progressLabel') }}</th>
<th>{{ coreString('scoreLabel') }}</th>
</template>
<template #tbody>
<transition-group
tag="tbody"
name="list"
>
<tr
v-for="tableRow in examsTable"
:key="tableRow.id"
<tr
v-for="tableRow in lessonsTable"
:key="tableRow.id"
>
<td>
<KRouterLink
:to="classRoute('ReportsLearnerReportLessonPage', {
lessonId: tableRow.id
})"
:text="tableRow.title"
icon="lesson"
/>
</td>
<td>
<StatusSimple :status="tableRow.status" />
</td>
</tr>
</transition-group>
</template>
</CoreTable>
</KGridItem>
<KGridItem :layout12="{ span: $isPrint ? 12 : 6 }">
<h2>{{ coachString('quizzesAssignedLabel') }}</h2>
<CoreTable
:class="{ print: $isPrint }"
:emptyMessage="coachString('quizListEmptyState')"
>
<template #headers>
<th>{{ coachString('titleLabel') }}</th>
<th>{{ coreString('progressLabel') }}</th>
<th>{{ coreString('scoreLabel') }}</th>
</template>
<template #tbody>
<transition-group
tag="tbody"
name="list"
>
<td>
<KRouterLink
:to="quizLink(tableRow.id)"
:text="tableRow.title"
icon="quiz"
/>
</td>
<td>
<StatusSimple :status="tableRow.statusObj.status" />
</td>
<td>
<Score :value="tableRow.statusObj.score" />
</td>
</tr>
</transition-group>
</template>
</CoreTable>
</KGridItem>
</KGrid>

<tr
v-for="tableRow in examsTable"
:key="tableRow.id"
>
<td>
<KRouterLink
:to="quizLink(tableRow.id)"
:text="tableRow.title"
icon="quiz"
/>
</td>
<td>
<StatusSimple :status="tableRow.statusObj.status" />
</td>
<td>
<Score :value="tableRow.statusObj.score" />
</td>
</tr>
</transition-group>
</template>
</CoreTable>
</KGridItem>
</KGrid>

</KTabsPanel>
</KPageContainer>
</CoachAppBarPage>

Expand All @@ -98,6 +102,7 @@
import commonCoach from '../common';
import CoachAppBarPage from '../CoachAppBarPage';
import { PageNames } from '../../constants';
import { LEARNERS_TABS_ID, LearnersTabs } from '../../constants/tabsConstants';
import ReportsLearnerHeader from './ReportsLearnerHeader';
import ReportsControls from './ReportsControls';

Expand All @@ -109,6 +114,12 @@
ReportsControls,
},
mixins: [commonCoach, commonCoreStrings],
data() {
return {
LEARNERS_TABS_ID,
LearnersTabs,
};
},
computed: {
learner() {
return this.learnerMap[this.$route.params.learnerId];
Expand Down