Closed yiwen101 closed 2 months ago
Hi @yiwen101 I think you shouldn;t be commiting the changes in the binary files
Hi @yiwen101 I think you shouldn;t be commiting the changes in the binary files
Thanks, will revert the result of npm run build:web
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 51.14%. Comparing base (
574e800
) to head (7ac9991
).:exclamation: Current head 7ac9991 differs from pull request most recent head 8940f7f. Consider uploading reports for the commit 8940f7f to get more accurate results
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Could you add a functional test for testing the behaviour described in the issue? If you can do a unit test, would be even better but might be a bit tough
Sorry for the late reply. I have added the functional test. To verify the result of fix, remember to do "markbind serve -d", otherwise the position will still be off(since frontend is updated per release).
LGTM!
The language for commit message seems off. how bout
Fix off-positioned close button in imported modal
Thanks for the review and suggestions on the commitment message. Yes, the suggested commit message is clearer. I have also updated the "proposed commit message" for this PR accordingly.
What is the purpose of this pull request?
Overview of changes: Fixes #2473 Credit to @tlylt for all the work and investigation; the actual fix code is trivial.
Anything you'd like to highlight/discuss:
Testing instructions: test with
markbind serve -d
Proposed commit message: (wrap lines at 72 characters) Fix off-positioned close button in imported modal
Checklist: :ballot_box_with_check:
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major
,r.Minor
,r.Patch
.Breaking change release note preparation (if applicable):