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

lint: Add support for CSS stylesheets in linting workflow. #66

Merged
merged 40 commits into from
Sep 18, 2024
Merged

lint: Add support for CSS stylesheets in linting workflow. #66

Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
24a30ab
Add Stylelint and Prettier configuration for CSS stylesheets.
junhaoliao Sep 10, 2024
653e71b
Update lint scripts to include CSS checks and fixes.
junhaoliao Sep 10, 2024
c1e9aa6
Add GitHub action problem matcher for Stylelint.
junhaoliao Sep 10, 2024
64a751c
Fix file-matcher remove path.
junhaoliao Sep 10, 2024
c95c20c
Remove path from `remove-matcher`.
junhaoliao Sep 10, 2024
308c319
Move `continue-on-error` to job meta to allow the job to be marked as…
junhaoliao Sep 10, 2024
8cd6e29
Add back `continue-on-error` on "npm --prefix new-log-viewer/ run lin…
junhaoliao Sep 10, 2024
b71d699
typo: Add back `continue-on-error` on "npm --prefix new-log-viewer/ r…
junhaoliao Sep 10, 2024
b09f16e
Remove `continue-on-error` and enhance conditional lint checks.
junhaoliao Sep 10, 2024
12a8bab
Always run "ataylorme/eslint-annotate-action@v3".
junhaoliao Sep 10, 2024
2c3c242
Add working directory for "Register Stylelint problem matcher".
junhaoliao Sep 10, 2024
676e929
Move working directory to `run: "npm run lint:css-check"`.
junhaoliao Sep 10, 2024
0a18614
Fix path in "Register Stylelint problem matcher"
junhaoliao Sep 10, 2024
e14fd6e
Wrap names with quotes.
junhaoliao Sep 10, 2024
78a859a
Remove unused npm script `lint:ci`.
junhaoliao Sep 10, 2024
764ab4f
Use sed to rename new-log-viewer paths.
junhaoliao Sep 10, 2024
d07d9b3
Replace single quote with double for sed.
junhaoliao Sep 10, 2024
c1ab8fd
Do sed on stderr as well.
junhaoliao Sep 10, 2024
7706e45
Fix lint step to correctly exit on error.
junhaoliao Sep 10, 2024
c7b50fc
Allow match if there's no leading space?
junhaoliao Sep 10, 2024
3de697f
Mark line numbers mandatory.
junhaoliao Sep 10, 2024
bb9307d
Correct indices.
junhaoliao Sep 10, 2024
ccc8a47
Fix regexp.
junhaoliao Sep 10, 2024
8f420b4
Remove color from stylelint output.
junhaoliao Sep 10, 2024
bafc21b
Make leading white spaces optional.
junhaoliao Sep 11, 2024
dae94ae
change pattern
junhaoliao Sep 11, 2024
4268be5
add severity.
junhaoliao Sep 11, 2024
94387dc
use github formatter instead
junhaoliao Sep 11, 2024
bf6b072
Lint stylesheets.
junhaoliao Sep 11, 2024
64b465a
Remove unused problem matcher.
junhaoliao Sep 11, 2024
64e7197
Add names to lint step actions.
junhaoliao Sep 11, 2024
009f24d
Add back npm script "npm run lint:check".
junhaoliao Sep 11, 2024
5481538
Merge branch 'main' into lint-css
junhaoliao Sep 11, 2024
0c6fa4f
Merge branch 'main' into lint-css
junhaoliao Sep 16, 2024
52beffc
Reformat CSS files.
junhaoliao Sep 16, 2024
b6db9a1
Merge branch 'y-scope:main' into lint-css
junhaoliao Sep 16, 2024
dbd2e87
Simplify lint scripts - Apply suggestions from code review
junhaoliao Sep 18, 2024
c597457
Update lint script configurations.
junhaoliao Sep 18, 2024
c0d4b71
Simplify lint workflow.
junhaoliao Sep 18, 2024
fff19ae
Update lint script and GitHub workflow for CI.
junhaoliao Sep 18, 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
2 changes: 2 additions & 0 deletions .github/workflows/lint.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@ jobs:
with:
node-version: 22
- run: "npm --prefix new-log-viewer/ clean-install"
- run: "npm --prefix new-log-viewer/ run lint:css-ci"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just update this now to call lint:check:js and lint:check:css so lint:check dosen't run lint:css twice

- run: "npm --prefix new-log-viewer/ run lint:check"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry I think this was supposed to be lint:js-check.

Suggested change
- run: "npm --prefix new-log-viewer/ run lint:check"
- run: "npm --prefix new-log-viewer/ run lint:js-check"

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 you rid of this later anyways. so i'm ignoring

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah it turn out that no changes are needed in the workflow file after the scripts are simplified

if: "always()"
6 changes: 6 additions & 0 deletions new-log-viewer/.prettierrc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
tabWidth: 4
useTabs: false
singleQuote: false
quoteProps: consistent
printWidth: 100
endOfLine: lf
7 changes: 7 additions & 0 deletions new-log-viewer/.stylelintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": [
"stylelint-config-standard",
"stylelint-config-clean-order/error",
"stylelint-prettier/recommended"
]
}
1,402 changes: 1,163 additions & 239 deletions new-log-viewer/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

19 changes: 16 additions & 3 deletions new-log-viewer/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@
"main": "src/index.tsx",
"scripts": {
"build": "webpack --config webpack.prod.js",
"start": "webpack serve --open --config webpack.dev.js",

"lint": "npm run lint:check",
"lint:check": "eslint src webpack.*.js --max-warnings 0",
"lint:fix": "npm run lint:check -- --fix",
"start": "webpack serve --open --config webpack.dev.js"
"lint:check": "npm-run-all --sequential --continue-on-error lint:css-check lint:js-check",
"lint:fix": "npm-run-all --sequential --continue-on-error lint:css-fix lint:js-fix",

"lint:css-check": "stylelint src/**/*.css",
"lint:css-ci": "npm run lint:css-check -- --formatter github",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Although the GitHub formatter has been deprecated according to the Stylelint docs [1], the default string formatter does not work with GitHub action problem matchers because the formatter outputs relative paths instead of absolute paths of the source files. There has been calls for behaviour change about resolving the paths of problem matcher since its debut, but the proposals have yet to be implemented [2]. Before we move the new-log-viewer files out to the project root, using the github formatter might be the quickest way.

[1] https://stylelint.io/user-guide/options#formatter
[2] actions/runner#765

Copy link
Contributor

Choose a reason for hiding this comment

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

I did some internet sloothing and found they plan to remove completely [1] [2]
) and recommend to just use third party extension. I made that change in my suggestion in package.json. It also removed deprecation warning

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://www.npmjs.com/package/stylelint-actions-formatters#:~:text=This%20package%20should%20be%20used%20with%20the%20stylelint%2Dproblem%2Dmatcher%20action.

This package should be used with the stylelint-problem-matcher action.

It seems that to get the whole flow working, we also need to use a custom GitHub action in the workflow, which can be less ideal.

Do you think we can keep using the github formatter for now? I have already had a Stylelint problem matcher drafted which works perfectly when we run Stylelint with the default string formatter from the project root. Hopefully in the coming days, when we move the files from /new-log-viewer into the project root, we can replace this github formatter with the string formatter and start using the problem matcher. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with that. I'm also happy to just use the string formatter now.. It looks like it still returns an error code even with string. So i don't mind just l opening the logs if there is an error.
Screenshot 2024-09-18 at 10 46 48 AM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think having the errors annotated inline would be more convenient. On the other hand, using a deprecated but not yet removed formatter isn't too harmful until we do version upgrades. We can continue the discussion at #66 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

see other comment

"lint:css-fix": "npm run lint:css-check -- --fix",
"lint:js-check": "eslint src webpack.*.js --max-warnings 0",
"lint:js-fix": "npm run lint:js-check -- --fix"
junhaoliao marked this conversation as resolved.
Show resolved Hide resolved
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -49,8 +56,14 @@
"html-webpack-plugin": "^5.6.0",
"mini-css-extract-plugin": "^2.9.0",
"monaco-editor-webpack-plugin": "^7.1.0",
"npm-run-all": "^4.1.5",
"prettier": "^3.3.3",
"react-refresh": "^0.14.2",
"style-loader": "^4.0.0",
"stylelint": "^16.9.0",
"stylelint-config-clean-order": "^6.1.0",
"stylelint-config-standard": "^36.0.1",
"stylelint-prettier": "^5.0.2",
"typescript": "^5.6.2",
"webpack": "^5.92.0",
"webpack-cli": "^5.1.4",
Expand Down
20 changes: 13 additions & 7 deletions new-log-viewer/src/components/DropFileContainer/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,27 @@

.hover-mask {
position: absolute;
z-index: var(--ylv-drop-file-container-hover-mask-z-index);
top: 0;
width: 100%;
height: 100%;
background-color: rgba(2, 88, 168, 0.2);

display: flex;
align-items: center;
justify-content: center;
z-index: var(--ylv-drop-file-container-hover-mask-z-index);

width: 100%;
height: 100%;

background-color: rgb(2 88 168 / 20%);
}

.hover-message {
z-index: var(--ylv-drop-file-container-hover-message-z-index);

padding: 8px;
color: #616161;
font-size: 0.875rem;

font-family: var(--ylv-ui-font-family), sans-serif;
font-size: 0.875rem;
color: #616161;

background-color: #f3f3f3;
z-index: var(--ylv-drop-file-container-hover-message-z-index);
}
2 changes: 1 addition & 1 deletion new-log-viewer/src/components/MenuBar/PageNumInput.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.page-num-input input::-webkit-outer-spin-button,
.page-num-input input::-webkit-inner-spin-button {
-webkit-appearance: none;
margin: 0;
appearance: none;
}

.page-num-input-num-pages-text {
Expand Down
2 changes: 1 addition & 1 deletion new-log-viewer/src/components/MenuBar/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.menu-bar {
display: flex;
flex-direction: row;
height: var(--ylv-status-bar-height);
align-items: center;
height: var(--ylv-status-bar-height);
}
8 changes: 5 additions & 3 deletions new-log-viewer/src/components/StatusBar/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
.status-bar {
align-items: center;
position: absolute;
bottom: 0;

display: flex;
position: absolute;
align-items: center;

width: 100%;
}

.status-message {
padding-left: 8px;
flex-grow: 1;
padding-left: 8px;
}
4 changes: 2 additions & 2 deletions new-log-viewer/src/components/modals/SettingsModal/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
.settings-dialog-title {
display: flex;
justify-content: space-between;
align-items: center;
flex-wrap: wrap;
align-items: center;
justify-content: space-between;
}

.settings-dialog-title-text {
Expand Down
12 changes: 7 additions & 5 deletions new-log-viewer/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
html, body, #root {
margin: 0;
height: 100%;
html,
body,
#root {
width: 100%;
height: 100%;
margin: 0;
}

html {
Expand All @@ -10,8 +12,8 @@ html {

:root {
/* font-family globals */
--ylv-ui-font-family: -apple-system, BlinkMacSystemFont, system-ui, Ubuntu, "Droid Sans",
Roboto;
--ylv-ui-font-family: -apple-system, "BlinkMacSystemFont", system-ui, "Ubuntu", "Droid Sans",
"Roboto";

/* size globals */
--ylv-status-bar-height: 32px;
Expand Down
Loading