Skip to content

refactor(countdown): move streams into lua#4372

Merged
Rathoz merged 20 commits intomainfrom
countdown
Jul 15, 2024
Merged

refactor(countdown): move streams into lua#4372
Rathoz merged 20 commits intomainfrom
countdown

Conversation

@hjpalpha
Copy link
Collaborator

@hjpalpha hjpalpha commented Jun 26, 2024

Summary

As per discussion in #4363 refactor the countdown stuff in a way that moves the stream building out of js into lua.

Also allows to call the stream icon building from outside of Module:Countdown.

How did you test this change?

on the deprecated secret commons
https://*****.liquipedia.net/commons/User:Hjpalpha/countdown
(mind css for infobox icons is broken on that wiki, hence the icons looking badly)

hjpalpha added 3 commits June 26, 2024 18:06
- get rid of some anno warnings
- add display components (in prep for moving it from js to lua)
- add filter function
- allow 2nd stream for any platform
@Nadoxis
Copy link
Contributor

Nadoxis commented Jul 4, 2024

The rest looks ok to me, don't know all the specific behaviors like legacy and stuff tho, but I trust you on this ;)

@hjpalpha hjpalpha marked this pull request as ready for review July 5, 2024 16:26
@hjpalpha hjpalpha requested review from Nadoxis, iMarbot and mbergen July 5, 2024 16:26
@Rathoz
Copy link
Collaborator

Rathoz commented Jul 8, 2024

Is it still in draft? Considering the PR title

@Rathoz Rathoz requested a review from liquidely July 8, 2024 10:15
@hjpalpha hjpalpha changed the title draft: feat(Countdown): Refactor countdowns feat(Countdown): Refactor countdowns Jul 8, 2024
@hjpalpha hjpalpha requested a review from Rathoz July 8, 2024 10:45
@hjpalpha hjpalpha added the javascript Changes to JavaScript files label Jul 8, 2024
- let js add the ` - ` with the same criteria as before
- display streams if finished is not set but the time switch has already happened
--> it will again show the date together with the stream but without dash and countdown
@hjpalpha hjpalpha requested a review from Rathoz July 11, 2024 15:47
@Rathoz
Copy link
Collaborator

Rathoz commented Jul 12, 2024

  • display streams if finished is not set but the time switch has already happened

Would it not be better to hide the steams in JS for this scenario? That's at least what I had in mind

@hjpalpha
Copy link
Collaborator Author

hjpalpha commented Jul 12, 2024

  • display streams if finished is not set but the time switch has already happened

Would it not be better to hide the steams in JS for this scenario? That's at least what I had in mind

it requires quite a bit of changes though
the problem is that we basically kill the inner of the timer obj element in js

timerObjectNode.innerHTML = '';
timerObjectNode.appendChild( dateChild );

and rebuild it entirely there

in addition it mirrors current behaviour 1:1 now

we could leave it as oos for a future PR

@Rathoz Rathoz changed the title feat(Countdown): Refactor countdowns refactor(countdown): move streams into lua Jul 12, 2024
hjpalpha and others added 2 commits July 15, 2024 14:37
Co-authored-by: Rikard Blixt <rikardblixt@gmail.com>
@hjpalpha hjpalpha requested a review from Rathoz July 15, 2024 13:19
@hjpalpha hjpalpha requested a review from Rathoz July 15, 2024 14:48
Copy link
Collaborator

@Rathoz Rathoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hjpalpha hjpalpha requested a review from Rathoz July 15, 2024 15:12
Copy link
Collaborator

@Rathoz Rathoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Rathoz Rathoz merged commit 711b475 into main Jul 15, 2024
@Rathoz Rathoz deleted the countdown branch July 15, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: standard javascript Changes to JavaScript files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants