Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added astronomical clock #144

Merged
merged 4 commits into from
Feb 13, 2024
Merged

Added astronomical clock #144

merged 4 commits into from
Feb 13, 2024

Conversation

Mond1c
Copy link
Contributor

@Mond1c Mond1c commented Feb 9, 2024

No description provided.

return <TextWrap part={part}>
<ContestClock/>
<ContestClock
timeZone={tickerSettings.timeZone === "" ? null : tickerSettings.timeZone}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just make the default null or undefined instead of an empty string?

Copy link
Contributor Author

@Mond1c Mond1c Feb 11, 2024

Choose a reason for hiding this comment

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

But if we have such a situation: we set "timeZone" to utc. Then we set "timeZone" to an empty string. In tickerSettings "timeZone" is now equals to an empty string. So it's not null => we have InvalidDateAndTime. But we expected contest time.

We can change this part of code:

const dateTime = DateTime.now().setZone(event.target.value);
if (dateTime.isValid || event.target.value === "") {
    setErrorMessage("");
    onChangeFieldEventHandler(setEditData, "timeZone")(event);
} else {
    setErrorMessage(dateTime.invalidReason);
}

But I don't think this is a good idea, becase we need to create a new function instead of using onChangeFieldEventHandler. (This necessary for this because if event.target.value is an empty string we can change value to null).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm talking mostly about backend behaviour here. As far as i can see this line of code just changes an empty string from the backend to a null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I can do something like this:

override fun toMessage(): ClockTickerMessage {
    if (timeZone != null && timeZone.isEmpty()) {
        return ClockTickerMessage(ClockTickerSettings(part, periodMs, null))
    }
    return ClockTickerMessage(this)
} 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -727,6 +727,7 @@ export enum TickerPart {
export interface clock {
part: TickerPart;
periodMs: number;
timeZone?: string | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Now there is a strange type in the typescript.
But I guess we will fix it once we (cc @kunyavskiy) figure out a way to canonize them to all be either null or undefined.

@irdkwmnsb irdkwmnsb merged commit 9a119aa into icpc:main Feb 13, 2024
3 checks passed
MegaVerkruzo pushed a commit to MegaVerkruzo/live-v3 that referenced this pull request Apr 9, 2024
* added astronomical clock

* fix strange thing in api.ts

* backend: fix situation when timeZone is an empty string

* fixed: ticker settings for preset buttons
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants