Combine error and warning boxes that have the same message #8154
Conversation
eivindjahren
added
the
release-notes:user-impact
eivindjahren
force-pushed
the
combine_messages
branch
2 times, most recently
from
ab537a9 to
c1f3af0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8154 +/- ##
==========================================
+ Coverage 76.93% 86.58% +9.64%
==========================================
Files 383 384 +1
Lines 23813 23883 +70
Branches 625 636 +11
==========================================
+ Hits 18320 20678 +2358
+ Misses 5412 3132 -2280
+ Partials 81 73 -8 ☔ View full report in Codecov by Sentry. |
eivindjahren
force-pushed
the
combine_messages
branch
from
c1f3af0 to
ffe2b7f
Compare
| if self._expanded: | ||
| self.lbl.setText(self._collapsed_text()) | ||
| self._expand_collapse_label.setText(self._expand_link()) | ||
| self._expanded = False | ||
| else: | ||
| self.lbl.setText(self._expanded_text()) | ||
| self._expand_collapse_label.setText(self._hide_link()) | ||
| self._expanded = True |
There was a problem hiding this comment.
Could do even simpler, but maybe not as readable.
self._expanded = not self._expanded
| return ( | ||
| '<div style="font-size: 16px; line-height: 24px;">' | ||
| + f'<b style="color: {self._text_color}">' | ||
| + self._header | ||
| + "</b>" | ||
| + self._message | ||
| + location_paragraphs | ||
| + "</div>" | ||
| ) |
There was a problem hiding this comment.
Is this duplication of lines 101-109? Would it make things better to extract this as a separate formatting function?
There was a problem hiding this comment.
Extracted common text.
| location_paragraphs += ( | ||
| f'<p><b style="color: {self._text_color}">location:</b>{loc}</p>' | ||
| ) | ||
| first = False | ||
| else: | ||
| location_paragraphs += f"<p>{loc}</p>" |
There was a problem hiding this comment.
This is also be a candidate to extract as a separate formatting function?
I am unsure if the readability is better after that though, but at least we avoid differing implementations if that would ever happen.
There was a problem hiding this comment.
Not sure if I extracted the duplication that you wanted but did extract some.
Resolves #8151
Screencast.from.2024-06-14.12-03-22.webm
When applicable