Conversation
hjpalpha
left a comment
There was a problem hiding this comment.
lgtm modulo open comment
Co-authored-by: hjpalpha <75081997+hjpalpha@users.noreply.github.com>
Co-authored-by: hjpalpha <75081997+hjpalpha@users.noreply.github.com>
adding live / main as well as filter functionality
ElectricalBoy
left a comment
There was a problem hiding this comment.
reviewed on phone
| ---@return Widget? | ||
| function VRSStandings:render() |
There was a problem hiding this comment.
imo should be sliced into several helper methods for better maintainability & readability, e.g., separate helper method for table header
| return HtmlWidgets.Span{ | ||
| css = {display="inline-block", width="160px"}, | ||
| children = PlayerDisplay.InlinePlayer({player = player}) | ||
| } |
There was a problem hiding this comment.
why not use BlockPlayer?
Co-authored-by: SyntacticSalt <mail@mbergen.de>
Co-authored-by: SyntacticSalt <mail@mbergen.de>
Co-authored-by: SyntacticSalt <mbergent@uni-bremen.de> Co-authored-by: ElectricalBoy <15651807+ElectricalBoy@users.noreply.github.com>
Co-authored-by: ElectricalBoy <15651807+ElectricalBoy@users.noreply.github.com>
ElectricalBoy
left a comment
There was a problem hiding this comment.
I suggest throwing fetch / storage logic to a separate module entirely
Would this not just be alot of duplication? They currently share rendering logic |
no it is generally a good idea to parse input and fetch data and then push that into a data-struct it results in shorter and more structured modules |
You can still have fetch+storage in one module, but separated from the rendering logic. So you'd move _parse, _store, _fetch into a different module, which then can call the widget for display |
956ec42 to
d55b25b
Compare
Summary
@MischiefCS creates inofficial VRS based on LPDB data and adds it regularly.
This adds a display and allows to store/fetch it in/from LPDB for use on other pages, e.g. an excerpt on mainpage.
Eventually this might get replaced by the existing Ratings module.
How did you test this change?
Storage+Display https://liquipedia.net/counterstrike/User:SyntacticSalt
Fetch+Display https://liquipedia.net/counterstrike/User:SyntacticSalt/VRSFetch