Skip to content

Commit

Permalink
BC-5178 - Improve icon references (#3391)
Browse files Browse the repository at this point in the history
Provide a clearly defined set of icons under the path "@icons/material".
The import of icons from that path is enforced by a custom linter rule.

* add named path @iCons to tsconfig and webpack config
* add custom rules for material icon imports
* create eslint plugin in new lib directory and add it to eslintrc
* add eslint-plugin-schulcloud to dockerfile
* apply new material import rule, add missing icons
* remove prefix 'mdiCustom' for custom icons
* replace '$mdi', import material icons from '@icons/material'

---------
Co-authored-by: Uwe Ilgenstein <[email protected]>
  • Loading branch information
NFriedo authored Sep 6, 2024
1 parent c495d68 commit 5ab53c3
Show file tree
Hide file tree
Showing 164 changed files with 614 additions and 296 deletions.
2 changes: 2 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ module.exports = {
parserOptions: {
ecmaVersion: 2020,
},
plugins: ["schulcloud"],
rules: {
"schulcloud/material-icon-imports": "error",
"@typescript-eslint/no-explicit-any": "warn",
"no-console": process.env.NODE_ENV === "production" ? "off" : "warn",
"no-debugger": process.env.NODE_ENV === "production" ? "off" : "warn",
Expand Down
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ RUN apt update && apt install -y g++ libcairo2-dev libpango1.0-dev libjpeg-dev l
WORKDIR /app

COPY package.json package-lock.json ./
COPY lib/eslint-plugin-schulcloud ./lib/eslint-plugin-schulcloud
RUN npm ci

COPY babel.config.js .eslintrc.js LICENSE.md .prettierrc.js tsconfig.json tsconfig.build.json .eslintignore .prettierignore ./
Expand Down
1 change: 1 addition & 0 deletions config/webpack/webpack.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ module.exports = {
"@feature-news-form": getDir("src/modules/feature/news-form"),
"@feature-media-shelf": getDir("src/modules/feature/media-shelf"),
"@feature-room": getDir("src/modules/feature/room"),
"@icons": getDir("src/components/icons"),
"@ui-alert": getDir("src/modules/ui/alert"),
"@ui-board": getDir("src/modules/ui/board"),
"@ui-chip": getDir("src/modules/ui/chip"),
Expand Down
2 changes: 2 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const config = {
"^@page-(.*)$": "<rootDir>/src/modules/page/$1",
"^@ui-(.*)$": "<rootDir>/src/modules/ui/$1",
"^@util-(.*)$": "<rootDir>/src/modules/util/$1",
"^@icons(.*)$": "<rootDir>/src/components/icons/$1",
"^@/(.*)$": "<rootDir>/src/$1",
"^@@/(.*)$": "<rootDir>/$1",
},
Expand Down Expand Up @@ -55,6 +56,7 @@ const config = {
"<rootDir>/src/utils/**/*.(js|ts)",
"<rootDir>/src/composables/**/*.(js|ts)",
"<rootDir>/src/layouts/**/*.{js,ts,vue}",
"<rootDir>/lib/eslint-plugin-schulcloud/**/*.{js,ts,vue}",
// Exclude
"!<rootDir>/src/components/base/**/*",
"!<rootDir>/src/components/icons/**/*",
Expand Down
7 changes: 7 additions & 0 deletions lib/eslint-plugin-schulcloud/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const materialIconImportRule = require("./material-icon-imports");

module.exports = {
rules: {
"material-icon-imports": materialIconImportRule,
},
};
38 changes: 38 additions & 0 deletions lib/eslint-plugin-schulcloud/material-icon-imports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
module.exports = {
meta: {
type: "problem",
docs: {
description:
"Enforce that icons are imported from our own subset of icons instead of the full Material Icons library.",
},
messages: {
noDirectIconImport:
"Material icons should only be imported from '@icons/material'.",
},
fixable: "code",
},

create(context) {
return {
ImportDeclaration(node) {
const importPath = node.source.value;

node.specifiers.forEach((specifier) => {
const isMaterialIconImport =
specifier.type === "ImportSpecifier" &&
/^mdi[A-Z]/.test(specifier.imported.name); // match 'mdi' followed by a capital letter

if (isMaterialIconImport && importPath !== "@icons/material") {
context.report({
node: specifier,
messageId: "noDirectIconImport",
fix: (fixer) => {
return fixer.replaceText(node.source, '"@icons/material"');
},
});
}
});
},
};
},
};
44 changes: 44 additions & 0 deletions lib/eslint-plugin-schulcloud/material-icon-imports.unit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
"use strict";

const { RuleTester } = require("eslint");
const rule = require("./material-icon-imports.js");

const ruleTester = new RuleTester({
parserOptions: { ecmaVersion: 2020, sourceType: "module" },
});

ruleTester.run("material-icon-imports", rule, {
valid: [
{
code: `import { mdiCheck } from "@icons/material";`,
},
{
code: `import { useI18n } from "vue-i18n";`,
},
{
code: `import { mdi } from "vuetify/iconsets/mdi-svg";`,
},
],
invalid: [
{
code: `import { mdiCheck } from "@mdi/js";`,
errors: [{ messageId: "noDirectIconImport" }],
output: `import { mdiCheck } from "@icons/material";`,
},
{
code: `import { mdiAlert } from "@/components/icons/material";`,
errors: [{ messageId: "noDirectIconImport" }],
output: `import { mdiAlert } from "@icons/material";`,
},
{
code: `import { mdiCheck } from "some-other-path";`,
errors: [{ messageId: "noDirectIconImport" }],
output: `import { mdiCheck } from "@icons/material";`,
},
{
code: `import { mdiCheck } from "../icons/material";`,
errors: [{ messageId: "noDirectIconImport" }],
output: `import { mdiCheck } from "@icons/material";`,
},
],
});
12 changes: 12 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"test:unit": "npx jest",
"test:unit:ci": "npm run test:unit -- --coverage --ci --maxWorkers=4",
"lint": "npx eslint 'src/**/*.{ts,js,vue}'",
"lint:fix": "npx eslint 'src/**/*.{ts,js,vue}' --fix",
"generate-client:server": "node generate-client.js -c openapitools-for-server.json",
"generate-client:filestorage": "node generate-client.js -u 'http://localhost:4444/api/v3/docs-json/' -p 'src/fileStorageApi/v3' -c 'openapitools-for-file-storage.json'",
"generate-client:h5p-editor": "node generate-client.js -u 'http://localhost:4448/api/v3/docs-json/' -p 'src/h5pEditorApi/v3' -c 'openapitools-for-h5p-editor.json'"
Expand Down Expand Up @@ -73,6 +74,7 @@
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-vue": "^9.23.0",
"eslint-plugin-schulcloud": "file:lib/eslint-plugin-schulcloud",
"eslint-webpack-plugin": "^4.1.0",
"fishery": "^2.2.2",
"html-webpack-plugin": "^5.6.0",
Expand Down
2 changes: 1 addition & 1 deletion src/components/administration/AdminMigrationSection.vue
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@
</template>

<script lang="ts">
import { mdiAlertCircle, mdiCheck } from "@/components/icons/material";
import { mdiAlertCircle, mdiCheck } from "@icons/material";
import { useUserLoginMigrationMappings } from "@/composables/user-login-migration-mappings.composable";
import { BusinessError } from "@/store/types/commons";
import { School } from "@/store/types/schools";
Expand Down
2 changes: 1 addition & 1 deletion src/components/administration/ExternalToolSection.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from "@@/tests/test-utils/setup";
import { useSchoolExternalToolUsage } from "@data-external-tool";
import { createMock, DeepMocked } from "@golevelup/ts-jest";
import { mdiAlert, mdiCheckCircle } from "@mdi/js";
import { mdiAlert, mdiCheckCircle } from "@icons/material";
import { mount, VueWrapper } from "@vue/test-utils";
import { nextTick, ref } from "vue";
import vueDompurifyHTMLPlugin from "vue-dompurify-html";
Expand Down
2 changes: 1 addition & 1 deletion src/components/administration/ExternalToolSection.vue
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ import {
} from "@/utils/inject";
import { useSchoolExternalToolUsage } from "@data-external-tool";
import { RenderHTML } from "@feature-render-html";
import { mdiAlert, mdiCheckCircle } from "@mdi/js";
import { mdiAlert, mdiCheckCircle } from "@icons/material";
import {
computed,
ComputedRef,
Expand Down
2 changes: 1 addition & 1 deletion src/components/administration/ExternalToolToolbar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import {
mdiPencilOutline,
mdiTrashCanOutline,
mdiFileDocumentOutline,
} from "@mdi/js";
} from "@icons/material";
defineEmits(["edit", "datasheet", "delete"]);
</script>
2 changes: 1 addition & 1 deletion src/components/atoms/InfoMessage.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</div>
</template>
<script>
import { mdiAlert } from "@/components/icons/material";
import { mdiAlert } from "@icons/material";
export default {
props: {
message: {
Expand Down
2 changes: 1 addition & 1 deletion src/components/atoms/VCustomChipTimeRemaining.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<script>
import { fromNowToFuture } from "@/plugins/datetime";
import dayjs from "dayjs";
import { mdiTimerSandComplete } from "@mdi/js";
import { mdiTimerSandComplete } from "@icons/material";
export default {
props: {
Expand Down
2 changes: 1 addition & 1 deletion src/components/atoms/vCustomBreadcrumbs.vue
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</template>

<script setup lang="ts">
import { mdiChevronLeft } from "@mdi/js";
import { mdiChevronLeft } from "@icons/material";
defineProps({
breadcrumbs: {
Expand Down
2 changes: 1 addition & 1 deletion src/components/atoms/vRoomAvatar.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
createTestingVuetify,
} from "@@/tests/test-utils/setup";
import { createMock } from "@golevelup/ts-jest";
import { mdiSync } from "@mdi/js";
import { mdiSync } from "@icons/material";
import { mount } from "@vue/test-utils";
import { VBadge } from "vuetify/lib/components/index.mjs";
import vRoomAvatar from "./vRoomAvatar.vue";
Expand Down
2 changes: 1 addition & 1 deletion src/components/atoms/vRoomAvatar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@

<script setup lang="ts">
import { computed, ref } from "vue";
import { mdiLock, mdiSync } from "@/components/icons/material";
import { mdiLock, mdiSync } from "@icons/material";
import { useI18n } from "vue-i18n";
import { useRouter } from "vue-router";
Expand Down
2 changes: 1 addition & 1 deletion src/components/atoms/vRoomEmptyAvatar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</div>
</template>
<script>
import { mdiLock } from "@/components/icons/material";
import { mdiLock } from "@icons/material";
export default {
props: {
size: {
Expand Down
11 changes: 8 additions & 3 deletions src/components/base/BaseInput/BaseInputCheckbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
</label>
</template>
<script>
import {
mdiCheckboxBlankOutline,
mdiCheckboxIntermediate,
mdiCheckboxOutline,
} from "@icons/material";
export const supportedTypes = ["checkbox"];
export default {
Expand Down Expand Up @@ -62,11 +67,11 @@ export default {
},
visibleIcon() {
if (this.showUndefinedState && this.modelValue === undefined) {
return { name: "$mdiCheckboxIntermediate" };
return { name: mdiCheckboxIntermediate };
}
return this.isChecked
? { name: "$mdiCheckboxOutline" }
: { name: "$mdiCheckboxBlankOutline" };
? { name: mdiCheckboxOutline }
: { name: mdiCheckboxBlankOutline };
},
},
created() {
Expand Down
25 changes: 18 additions & 7 deletions src/components/base/BaseInput/BaseInputDefault.vue
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,16 @@
>
<v-icon>{{ visibilityIcon }}</v-icon>
</v-btn>
<v-icon v-if="hasError" color="rgba(var(--v-theme-error))"
>$mdiAlert</v-icon
>
<v-icon v-if="success" color="rgba(var(--v-theme-success))">
$mdiCheckCircleOutline
</v-icon>
<v-icon
v-if="hasError"
color="rgba(var(--v-theme-error))"
:icon="mdiAlert"
/>
<v-icon
v-if="success"
color="rgba(var(--v-theme-success))"
:icon="mdiCheckCircleOutline"
/>
</div>
</div>
</div>
Expand All @@ -88,7 +92,12 @@
</template>
<script>
import { inputRangeDate } from "@/plugins/datetime";
import { mdiEyeOffOutline, mdiEyeOutline } from "@mdi/js";
import {
mdiAlert,
mdiCheckCircleOutline,
mdiEyeOffOutline,
mdiEyeOutline,
} from "@icons/material";
import { defineComponent } from "vue";
import { useUid } from "@/utils/uid";
Expand Down Expand Up @@ -144,6 +153,8 @@ export default defineComponent({
},
data() {
return {
mdiAlert,
mdiCheckCircleOutline,
mdiEyeOffOutline,
mdiEyeOutline,
hasFocus: false,
Expand Down
2 changes: 1 addition & 1 deletion src/components/copy-result-modal/CopyResultModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import {
mdiCheckCircle,
mdiCloseCircle,
mdiInformation,
} from "@mdi/js";
} from "@icons/material";
export default {
name: "CopyResultModal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

<script>
import { CopyApiResponseTypeEnum } from "@/serverApi/v3";
import { mdiChevronLeft, mdiMenuRight } from "@mdi/js";
import { mdiChevronLeft, mdiMenuRight } from "@icons/material";
export default {
name: "CopyResultModalListItem",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@

<script setup lang="ts">
import ExternalToolConfigSettings from "@/components/external-tools/configuration/ExternalToolConfigSettings.vue";
import { mdiAlertCircle } from "@/components/icons/material";
import { mdiAlertCircle, mdiContentPaste } from "@icons/material";
import { useExternalToolMappings } from "@/composables/external-tool-mappings.composable";
import {
SchoolExternalTool,
Expand All @@ -114,7 +114,6 @@ import {
ContextExternalTool,
ExternalToolConfigurationTemplate,
} from "@data-external-tool";
import { mdiContentPaste } from "@mdi/js";
import {
computed,
ComputedRef,
Expand Down
Loading

0 comments on commit 5ab53c3

Please sign in to comment.