-
Notifications
You must be signed in to change notification settings - Fork 58
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.
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
Enable and upgrade stylelint
in pre-commit
#1388
Conversation
d17e15f
to
f901dcc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checklist for QA:
- I have checked out this branch, and successfully ran a fresh
make reset
. - I confirmed that there are no unintended functional regressions in this branch:
- I have managed to pass the onboarding flow
- Objects and Findings are created properly
- Tasks are created and completed properly
- I confirmed that the PR's advertised
feature
orhotfix
works as intended.
What works:
General functionality excluding styling.
What doesn't work:
Header is broken:
Crisis room layout is weird:
And other styling issues.
Bug or feature?:
- bullet point + screenshot (if useful) if it is unclear whether something is a bug or an intended feature.
@praseodym can you verify if this persists when you run Edit: When I do this, I see the correct styling for the header. The duplicate submit buttons isn't correct but also isn't regression, since it's been that way for a while now, so I would like to address that in another ticket. |
@praseodym and I just verified that the discrepancy is caused by old |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checked out a clean copy of this branch, the header is still broken.
@HeleenSG JavaScript is indeed still broken, I see the error |
I have just checked out this branch and can't reproduce any of the errors mentioned here before. I've cleared the Parcel cache and can compile and load |
# Conflicts: # .pre-commit-config.yaml
Signed-off-by: ammar <[email protected]>
You probably have some old assets still left over in your Django static directory. After building 4c481e2 in a completely clean environment, |
Signed-off-by: ammar <[email protected]>
You're right, I managed to reproduce this with a freshly checked-out repository. After some debugging and discussion with @HeleenSG we came to the conclusion that |
Changes
It was planned to integrate
stylelint
as part of the Manon upgrade. The pipeline was implemented, but actually fixing the styling issues has not been done yet. This PR upgrades and enables it.I'd like to request the frontend-gods @TwistMeister or @HeleenSG to perform the actual fixes :)
Running
pre-commit run --all-files
should already do most of the work.Issue link
Originally mentioned in #548 and #719.
Proof
See CI.
Code Checklist
Communication
.env
changes files if required and changed the.env-dist
accordingly.Checklist for code reviewers:
Copy-paste the checklist from the docs/source/templates folder into your comment.
Checklist for QA:
Copy-paste the checklist from the docs/source/templates folder into your comment.