mlomb / chat-analytics

Generate interactive, beautiful and insightful chat analysis reports
https://chatanalytics.app
GNU Affero General Public License v3.0
711 stars 51 forks source link

Some refactoring and SPaG fixes #95

Closed hopperelec closed 11 months ago

hopperelec commented 1 year ago

Edit: Didn't mean to include https://github.com/mlomb/chat-analytics/pull/94 in this, but I'll probably just cause issues with merging if I try to remove it

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8bcf36e) 71.93% compared to head (5350c3b) 71.90%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #95 +/- ## ========================================== - Coverage 71.93% 71.90% -0.03% ========================================== Files 60 60 Lines 2444 2442 -2 Branches 514 514 ========================================== - Hits 1758 1756 -2 Misses 626 626 Partials 60 60 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 1 year ago

⚡ Preview for this PR: https://pr-95.chat-analytics.pages.dev 📊 Demo

mlomb commented 1 year ago

By the way, what do you mean with "SPaG"? 🤔

hopperelec commented 1 year ago

By the way, what do you mean with "SPaG"? 🤔

Spelling, Punctuation and Grammar. Sorry, probably just a UK thing lol

mlomb commented 11 months ago

I carefully checked all the changes and I'm ready to merge!


I added the following again:

fieldset {
  display: grid;
  display: contents;
}

Which was necessary for the Stepper (homepage, while processing) to display correctly.


I rolled back changes in BitStream:

value >>>= 7; // from this
value = value >>> 7; // back to this

I don't recall correctly and can't find a source but I had problems with >>>= in some browsers (iirc older but not dead versions of Safari), so I prefer to keep it that way


Thanks for your PR once again! Lot of work, thanks!! 🎊🎊🎊

hopperelec commented 11 months ago

Hm I did definitely test the stepper a lot and didn't see any issues, I don't know what having two display properties is supposed to do. Of will. But that older browser issue makes sense, no worries!

mlomb commented 11 months ago

having two display properties is supposed to do

iirc it was a fallback