refactor: Use template literals instead of string concatenation when building popup HTML#1464
refactor: Use template literals instead of string concatenation when building popup HTML#1464tora-pan wants to merge 3 commits intomelink14:mainfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1464 +/- ##
==========================================
+ Coverage 79.62% 79.86% +0.23%
==========================================
Files 7 7
Lines 3004 3005 +1
Branches 189 189
==========================================
+ Hits 2392 2400 +8
+ Misses 607 600 -7
Partials 5 5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
melink14
left a comment
There was a problem hiding this comment.
This seems fine overall but will need to take a block of time to study it since it produced a non-trivial diff.
Since several of the branches don't have tests (not your fault) we might want to add those first so we can be confident in the refactor. (I can do it as well if you don't have time or desire)
Finally, I agree the variable renaming is good but I would probably have done that in it's own PR first since it leads to cleaner diffs and easier to review PR. That said, it's only a minor inconvenience in this case so it's not worth struggling with git to split it! It's something to consider in the future though!
I also updated the title and body a little bit to have the title focus on exactly what was done and the body give extra context for why (and thanks for separately mentioning renames! that's very important when combining changes!). It was very close already though so only needed minor tweaks ('need' is a strong word here)
The inline variable replacement of template literals allows us to remain the desired HTML shape of the string.
Renamed variables for better readability.