feat(MatchTicker): Adding new match ticker component style #4363
Conversation
| local NOW = os.date('%Y-%m-%d %H:%M', os.time(os.date('!*t') --[[@as osdateparam]])) | ||
|
|
||
| ---Display class for the header of a match ticker | ||
| ---@class MatchTickerHeader |
There was a problem hiding this comment.
fyi defining the class with the same name as in the old version will cause annotation warnings ;)
| for platformName, targetStream in pairs(streams) do | ||
| local streamLink = mw.ext.StreamPage.resolve_stream(platformName, targetStream) | ||
|
|
||
| if streamLink then | ||
| -- Default values | ||
| local url = 'Special:Stream/' .. platformName .. '/' .. streamLink | ||
| local icon = '<i class="lp-icon lp-icon-21 lp-' .. platformName .. '"></i>' | ||
|
|
||
| -- TL.net specific | ||
| if platformName == 'stream' then | ||
| url = 'https://tl.net/video/streams/' .. streamLink | ||
| end | ||
|
|
||
| streamLinks = streamLinks .. '[[' .. url .. '|' .. icon .. ']]' | ||
| end | ||
| end | ||
|
|
||
| links:wikitext(streamLinks) |
There was a problem hiding this comment.
Don't know if this is the correct way of doing stream links. Currently, the logic is in the Countdown.js logic but after talking to FO, we decided that it could be done on the Lua side of things. Could maybe lie somewhere else.
There was a problem hiding this comment.
imo we should move it into module countdown or a new module then so it is accessible easily from other places
could also rewrite the countdown module for it
mind there is a PR to rewrite countdown module anyways with #3111
if you want i can rework that old PR (split it into several parts) and integrate the stream link building there (as sep. function) thursday/friday
There was a problem hiding this comment.
If you're willing to do it, I'm fine with it. Tell me if I can help with that
There was a problem hiding this comment.
Shouldn't we put this into https://github.com/Liquipedia/Lua-Modules/blob/main/standard/links_stream.lua?
There was a problem hiding this comment.
Agree, and the countdown module can use it
There was a problem hiding this comment.
Just saw it is already in here #4372
KilMer56
changed the title
There was a problem hiding this comment.
what is the placeholder on the right supposed to show?
also i personally think the winner should be highlighted somehow to make clear it is the winner
(personally i am fine with kicking the gradient bg stuff fwiw, just think that some indication should be given)
overall i like the new design, looks clean :)
components/match_ticker/commons/match_ticker_display_components_new.lua
Outdated
Show resolved
Hide resolved
components/match_ticker/commons/match_ticker_display_components_new.lua
Outdated
Show resolved
Hide resolved
| url = 'https://tl.net/video/streams/' .. streamLink | ||
| end | ||
|
|
||
| streamLinks = streamLinks .. '[[' .. url .. '|' .. icon .. ']]' |
There was a problem hiding this comment.
could use module:page here (can be ignored if we first do the countdown update)
also i think the js adds a space between the icons
also doesn't support 'stream' (needs external link, this only uses internal ones)
There was a problem hiding this comment.
The space between the icons is dealt with in the CSS
For the stream yes good catch, can update it depending on when we want to do the countdown update
components/match_ticker/commons/match_ticker_display_components_new.lua
Outdated
Show resolved
Hide resolved
| function Match:create() | ||
| self.root:node(self:standardMatchRow()) | ||
| self.root:node(self:detailsRow()) | ||
|
|
||
| return self.root | ||
| end |
There was a problem hiding this comment.
just as fyi: we are dropping BR support here, which atm is irrelevant since we don't have any BR support in match2 yet (except some edge cases on sc2 which i would want to exclude on match ticker anyways :P)
|
The placeholder is supposed to show either the date or the countdown. Here it shows nothing since the matches in devs are old and not marked as finished, will give some tests on live to see if it behaves correctly. For the winner, you're right I will put the winner in bold for now and we can see later on to improve the visibility over this! |
stylesheets/commons/MatchTicker.less
Outdated
| gap: 0.25rem; | ||
| } | ||
|
|
||
| .timer-hidden { |
There was a problem hiding this comment.
If this is a modifier class, you'd want to write it as timer--hidden.
There was a problem hiding this comment.
Removed it as it should be in the other MR
| border-radius: 0.25rem; | ||
| font-weight: bold; | ||
|
|
||
| &.timer-object-countdown-live { |
There was a problem hiding this comment.
You could simplify this to countdown--live or is--live
There was a problem hiding this comment.
This is because Countdown.js is using that class. Will update it in the other MR then
Summary
This is setting up the new Match Ticker structure and visuals we want to use for the Dota2 Main Page. It will most likely be extended to other pages and wikis, but the scope of this one is only this context.
How did you test this change?
In my dev environment: http://killian.wiki.tldev.eu/rocketleague/index.php?title=Template:GameList/Dev
Currently testing it on live to see how it would look like