jessevanmuijden
commented
3 weeks ago No need to use else. Just an early return is cleaner https://github.com/biz-dev-ops/web-components/blob/main/src/business-reference-architecture/architecture-button/abstract-button/index.ts#L26
It is always a good idea to add a readme to onboard new developers quicker https://github.com/biz-dev-ops/web-components/blob/main/README.md
Indent with consistent number of spaces instead of tabs to improve readability cross platform. Github displays a tab as 8 spaces. https://github.com/biz-dev-ops/web-components/blob/main/package.json#L2
Call a composite action for package publication here, so you can use the if-statement once and prevent duplication of the if-statement in the steps of the action https://github.com/biz-dev-ops/web-components/blob/main/.github/workflows/release.yml#L54
Minify-html-literals-loader is missing from devDependencies and used in web pack.config.json. I see in a recent PR that it will be removed entirely for security reasons. https://github.com/biz-dev-ops/web-components/blob/main/webpack.config.js#L23
Double space https://github.com/biz-dev-ops/web-components/blob/main/src/shared/popover/popover.css.ts#L22C70-L22C72
I would drop the underscore in --_ if possible https://github.com/biz-dev-ops/web-components/blob/main/src/shared/truncate/truncate.css.ts#L6
Might as well make it a one-liner if not using brackets https://github.com/biz-dev-ops/web-components/blob/main/src/shared/util/index.ts#L3
Inconsistent spacing, prettier could be used. https://github.com/biz-dev-ops/web-components/blob/main/src/use-case-viewer/index.ts#L92
Grid layout with css property gap may be cleaner https://github.com/biz-dev-ops/web-components/blob/main/src/use-case-viewer/use-case-viewer.css.ts#L73
Better to check for window being defined and use strict typing https://github.com/biz-dev-ops/web-components/blob/main/src/model-viewer/index.ts#L26
Why a 2 second timeout? That may be what slows down navigation. Maybe add a DOMContentLoaded event listener. https://github.com/biz-dev-ops/web-components/blob/main/src/model-viewer/index.html#L30
There is a lot of data (model) mixed into the template (view) here. Concerns can be separated better. https://github.com/biz-dev-ops/web-components/blob/main/src/model-viewer/index.html#L31
if ([‘description’, ‘title’, ’type’, ‘format’].includes(property) {
https://github.com/biz-dev-ops/web-components/blob/main/src/model-viewer/components/model-viewer-item/model-viewer-item-value/index.ts#L16
Is it possible to load the raw css from a css file instead of mixing it directly into the component? https://github.com/biz-dev-ops/web-components/blob/main/src/business-reference-architecture/architecture-button/default-button/index.ts#L12
I would refactor some of the many different implementations of abstract components that consist mainly of different styling to a single component and load different styles from a css component based on a variant prop.
I have checked out the project and found that the links are all broken on /business-reference-architecture Many of the pages and components are not responsive. Media queries and ellipsis can be used more. Navigation in development is slow.
arjangeertsema
Code review requested for the entire project: