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

BC-8578 - Remove Visibility Alert From Board #3516

Merged
merged 14 commits into from
Jan 27, 2025
Merged

BC-8578 - Remove Visibility Alert From Board #3516

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b1f072c
use vuetify lang key (it's the same)
odalys-dataport Jan 22, 2025
71a9247
add tooltip & remove alert
odalys-dataport Jan 22, 2025
dc6a69e
Merge branch 'main' into BC-8578-visibility-alert
odalys-dataport Jan 22, 2025
b623061
adjust unit tests
odalys-dataport Jan 22, 2025
2d3eb4a
Merge branch 'BC-8578-visibility-alert' of https://github.com/hpi-sch…
odalys-dataport Jan 22, 2025
98b3efe
capitalized draft translations
odalys-dataport Jan 23, 2025
a01b4b9
add DraftChip component
odalys-dataport Jan 23, 2025
0efab06
Merge branch 'main' into BC-8578-visibility-alert
odalys-dataport Jan 24, 2025
4364649
remove unused import
odalys-dataport Jan 24, 2025
72c319d
Merge branch 'main' into BC-8578-visibility-alert
odalys-dataport Jan 27, 2025
bdfc07a
adjust draft chip test
odalys-dataport Jan 27, 2025
93e44e9
Merge branch 'main' into BC-8578-visibility-alert
odalys-dataport Jan 27, 2025
d3f7850
Merge branch 'main' into BC-8578-visibility-alert
odalys-dataport Jan 27, 2025
63bf81b
Merge branch 'main' into BC-8578-visibility-alert
odalys-dataport Jan 27, 2025
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
1 change: 0 additions & 1 deletion src/components/molecules/Alert.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
closable
max-width="400"
min-width="200"
:close-label="$t('common.labels.close')"
uidp marked this conversation as resolved.
Show resolved Hide resolved
border="start"
@update:modelValue="closeNotification"
>
Expand Down
5 changes: 1 addition & 4 deletions src/locales/de.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,10 @@ export default {
"components.board.action.moveUp": "Nach oben verschieben",
"components.board.action.changeLayout": "Ansicht ändern",
"components.board.action.shareLink.card": "Link zur Karte kopieren",
"components.board.alert.info.teacher":
"Dieser Bereich ist sichtbar für alle Kursteilnehmenden.",
"components.board.alert.info.draft":
"Dieser Bereich ist nicht für die Kursteilnehmenden sichtbar.",
"components.board.column.defaultTitle": "Neuer Abschnitt",
"components.board.column.ghost.column.placeholder": "Abschnitt hinzufügen",
"components.board.column.ghost.list.placeholder": "Abschnitt hinzufügen",
"components.board.draftChip.tooltip": "Nur für Lehrkräfte sichtbar",
"components.board.error.404":
"Fehler 404 – Bereich mit diesem Namen wurde nicht gefunden",
"components.board.error.403":
Expand Down
9 changes: 3 additions & 6 deletions src/locales/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ export default {
"Link could not be copied to clipboard",
"common.words.courseGroups": "Course Groups",
"common.words.courses": "Courses",
"common.words.draft": "draft",
"common.words.drafts": "drafts",
"common.words.draft": "Draft",
"common.words.drafts": "Drafts",
"common.words.languages.de": "German",
"common.words.languages.en": "English",
"common.words.languages.es": "Spanish",
Expand Down Expand Up @@ -375,13 +375,10 @@ export default {
"components.board.action.moveUp": "Move up",
"components.board.action.changeLayout": "Change layout",
"components.board.action.shareLink.card": "Copy link to card",
"components.board.alert.info.teacher":
"This board is visible to all course participants.",
"components.board.alert.info.draft":
"This board is not visible to course participants.",
"components.board.column.defaultTitle": "New column",
"components.board.column.ghost.column.placeholder": "Add column",
"components.board.column.ghost.list.placeholder": "Add section",
"components.board.draftChip.tooltip": "Only visible to teachers",
"components.board.error.404":
"Error 404 – Board with this name was not found",
"components.board.error.403":
Expand Down
9 changes: 3 additions & 6 deletions src/locales/es.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ export default {
"El enlace no se pudo copiar al portapapeles",
"common.words.courseGroups": "grupos de cursos",
"common.words.courses": "Cursos",
"common.words.draft": "borrador",
"common.words.drafts": "borradores",
"common.words.draft": "Borrador",
"common.words.drafts": "Borradores",
"common.words.languages.de": "Alemán",
"common.words.languages.en": "Inglés",
"common.words.languages.es": "Español",
Expand Down Expand Up @@ -380,13 +380,10 @@ export default {
"components.board.action.moveUp": "Levantar",
"components.board.action.changeLayout": "Cambiar vista",
"components.board.action.shareLink.card": "Enlace a la ficha",
"components.board.alert.info.teacher":
"Este tablero es visible para todos los participantes en el curso.",
"components.board.alert.info.draft":
"Este tablón no es visible para los participantes en el curso.",
"components.board.column.defaultTitle": "Nueva columna",
"components.board.column.ghost.column.placeholder": "Añadir columna",
"components.board.column.ghost.list.placeholder": "Añadir sección",
"components.board.draftChip.tooltip": "Sólo visible para los profesores",
"components.board.error.404":
"Error 404 – No se ha encontrado ningún tablero con este nombre",
"components.board.error.403":
Expand Down
7 changes: 3 additions & 4 deletions src/locales/uk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ export default {
"Не вдалося скопіювати посилання в буфер обміну",
"common.words.courseGroups": "курсові групи",
"common.words.courses": "Мій курс",
"common.words.draft": "чернетка",
"common.words.drafts": "чернетки",
"common.words.draft": "Чернетка",
"common.words.drafts": "Чернетки",
"common.words.languages.de": "Німецька",
"common.words.languages.en": "Англійська",
"common.words.languages.es": "Іспанська",
Expand Down Expand Up @@ -384,11 +384,10 @@ export default {
"components.board.action.moveUp": "Рухатися вгору",
"components.board.action.changeLayout": "Змінити вигляд",
"components.board.action.shareLink.card": "Скопіювати посилання на Карту",
"components.board.alert.info.teacher": "Цю дошку бачать усі учасники курсу.",
"components.board.alert.info.draft": "Ця дошка невидима для учасників курсу.",
"components.board.column.defaultTitle": "Нова колонка",
"components.board.column.ghost.column.placeholder": "Додати стовпець",
"components.board.column.ghost.list.placeholder": "Додати розділ",
"components.board.draftChip.tooltip": "Видно лише вчителям",
"components.board.error.404":
"Помилка 404 – Дошки з такою назвою не знайдено",
"components.board.error.403":
Expand Down
84 changes: 0 additions & 84 deletions src/modules/feature/board/board/Board.unit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,90 +467,6 @@ describe("Board", () => {
});
});

describe("Info message for teacher", () => {
beforeEach(() => {
mockedBoardPermissions.isTeacher = true;
});

it("should call the board notifier when the user is teacher", () => {
jest.useFakeTimers();

setup();

jest.runAllTimers();

expect(mockedBoardNotifierCalls.showCustomNotifier).toHaveBeenCalled();
});

it("should call the board notifier with draft info when board is not visible", () => {
jest.useFakeTimers();

setup({ isBoardVisible: false });

jest.runAllTimers();

expect(mockedBoardNotifierCalls.showCustomNotifier).toHaveBeenCalledWith(
"components.board.alert.info.draft",
"info"
);
});

it("should call the board notifier with published info when board is visible", () => {
jest.useFakeTimers();

setup();

jest.runAllTimers();

expect(mockedBoardNotifierCalls.showCustomNotifier).toHaveBeenCalledWith(
"components.board.alert.info.teacher",
"info"
);
});

it("should call the board notifier with published info when board becomes visible", () => {
jest.useFakeTimers();

const { boardStore } = setup({ isBoardVisible: false });
boardStore.board!.isVisible = true;

jest.runAllTimers();

expect(mockedBoardNotifierCalls.showCustomNotifier).toHaveBeenCalledWith(
"components.board.alert.info.teacher",
"info"
);
});

it("should call the board notifier with draft info when board should not be visible anymore", () => {
jest.useFakeTimers();

const { boardStore } = setup();
boardStore.board!.isVisible = false;

jest.runAllTimers();

expect(mockedBoardNotifierCalls.showCustomNotifier).toHaveBeenCalledWith(
"components.board.alert.info.draft",
"info"
);
});

it("should not call the board notifier when the user is not a teacher", () => {
mockedBoardPermissions.isTeacher = false;

jest.useFakeTimers();

setup();

jest.runAllTimers();

expect(
mockedBoardNotifierCalls.showCustomNotifier
).not.toHaveBeenCalled();
});
});

describe("BoardColumnGhost component", () => {
describe("when user has create column permission", () => {
it("should not be rendered on DOM", () => {
Expand Down
18 changes: 0 additions & 18 deletions src/modules/feature/board/board/Board.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ const onUpdateBoardVisibility = async (isVisible: boolean) => {
boardId: props.boardId,
isVisible,
});
await setAlert();
};

const onUpdateColumnTitle = async (columnId: string, newTitle: string) => {
Expand All @@ -303,7 +302,6 @@ const scrollToNodeAndFocus = (scrollTargetId: string) => {

onMounted(async () => {
resetPageInformation();
setAlert();
useBoardInactivity();
const boardFetchPromise = boardStore.fetchBoardRequest({
boardId: props.boardId,
Expand All @@ -328,25 +326,9 @@ onUnmounted(() => {
resetNotifierModule();
});

const setAlert = useDebounceFn(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove import of useDebounceFn above

if (!isTeacher) return;

if (!board.value) {
return;
}

if (!isBoardVisible.value) {
showCustomNotifier(t("components.board.alert.info.draft"), "info");
} else {
showCustomNotifier(t("components.board.alert.info.teacher"), "info");
}
}, 150);

watch(
() => isBoardVisible.value,
() => {
setAlert();

if (!(isBoardVisible.value || isTeacher)) {
router.replace({ name: "room-details", params: { id: roomId.value } });
applicationErrorModule.setError(
Expand Down
22 changes: 22 additions & 0 deletions src/modules/feature/board/board/BoardDraftChip.unit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {
createTestingI18n,
createTestingVuetify,
} from "@@/tests/test-utils/setup";
import BoardDraftChip from "./BoardDraftChip.vue";

describe("@feature-board/BoardDraftChip", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think about two additional tests for text in VChip and VTooltip?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Imo this falls under the category "testing the framework" as there is no logic or changing texts in this component.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, maybe it would make sense to check if the correct text is rendered inside the component. Something like:

it("should render the text message", () => {
	const wrapper = setup();

	expect(wrapper.text()).toContain("common.words.draft");
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do. Not sure how easy it is to test for the tooltip text though

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. I confused the two components, sorry.

const setup = () => {
const wrapper = mount(BoardDraftChip, {
global: {
plugins: [createTestingI18n(), createTestingVuetify()],
},
});
return wrapper;
};

it("should render chip", () => {
const wrapper = setup();

expect(wrapper.findComponent({ name: "VChip" }).exists()).toBe(true);
});
});
14 changes: 14 additions & 0 deletions src/modules/feature/board/board/BoardDraftChip.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<template>
<VTooltip location="bottom" :text="$t('components.board.draftChip.tooltip')">
<template v-slot:activator="{ props }">
<VChip
v-bind="props"
size="small"
class="align-self-center cursor-default"
data-testid="board-draft-chip"
>
{{ $t("common.words.draft") }}
</VChip>
</template>
</VTooltip>
</template>
8 changes: 7 additions & 1 deletion src/modules/feature/board/board/BoardHeader.unit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ describe("BoardHeader", () => {
provide: {
[ENV_CONFIG_MODULE_KEY.valueOf()]: envConfigModuleMock,
},
stubs: {
VTooltip: false,
VOverlay: false,
},
},
props: {
title: "title-text",
Expand Down Expand Up @@ -308,7 +312,9 @@ describe("BoardHeader", () => {
{ isDraft: true }
);

expect(wrapper.findComponent({ name: "v-chip" }).exists()).toBe(true);
expect(wrapper.findComponent({ name: "BoardDraftChip" }).exists()).toBe(
true
);
});

it("should display 'publish' button instead of 'revert' button in menu", async () => {
Expand Down
10 changes: 2 additions & 8 deletions src/modules/feature/board/board/BoardHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,7 @@
</div>
</InlineEditInteractionHandler>
<div class="d-flex">
<v-chip
v-if="isDraft"
size="small"
class="align-self-center"
data-testid="board-draft-chip"
>
{{ t("common.words.draft") }}
</v-chip>
<BoardDraftChip v-if="isDraft" />
<div class="mx-2">
<BoardMenu
v-if="hasEditPermission"
Expand Down Expand Up @@ -76,6 +69,7 @@ import { computed, onMounted, ref, toRef, watchEffect } from "vue";
import { useI18n } from "vue-i18n";
import BoardAnyTitleInput from "../shared/BoardAnyTitleInput.vue";
import InlineEditInteractionHandler from "../shared/InlineEditInteractionHandler.vue";
import BoardDraftChip from "./BoardDraftChip.vue";

const props = defineProps({
boardId: {
Expand Down
Loading