doorstop-dev / doorstop

Requirements management using version control.
https://doorstop.readthedocs.io
Other
471 stars 132 forks source link

Fix html assets #612

Closed villanella closed 9 months ago

villanella commented 11 months ago

589 and #327 fixes

Details: For fixing #589 I wanted to copy template dir to output assets dir as a part of get_template function and it worked fine in manual tests, but this change broke some tests, so I decided to duplicate template dir as assets dir in doorstop/core/files instead of code changing. Maybe the variant from my first commit would be better, but it requires update of tests.

For fixing #327 (overlapping text by sidebar) I used the idea from this unmerged PR - https://github.com/doorstop-dev/doorstop/pull/353/files, I added 2 params in sidebar.css:

    width: 18em;
    overflow: hidden;
codecov-commenter commented 11 months ago

Codecov Report

Merging #612 (8dd5aad) into develop (e91dc8f) will decrease coverage by 0.03%. The diff coverage is n/a.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           develop     #612      +/-   ##
===========================================
- Coverage    94.95%   94.93%   -0.03%     
===========================================
  Files           36       36              
  Lines         4853     4853              
  Branches       930      930              
===========================================
- Hits          4608     4607       -1     
- Misses         148      149       +1     
  Partials        97       97              

see 1 file with indirect coverage changes

jacebrowning commented 11 months ago

Maybe the variant from my first commit would be better, but it requires update of tests.

I think that would be preferable to duplicating the asset files.

villanella commented 11 months ago

Maybe the variant from my first commit would be better, but it requires update of tests.

I think that would be preferable to duplicating the asset files.

@jacebrowning Thank you for your response, in my PR this is done.

neerdoc commented 9 months ago

I think we can close this. #589 is fixed already, and I have a PR soon that fixes #327 as well. I just have to add a few more test cases.