Update how splits are loaded from the leaderboard #363
Conversation
|
@CryZe What do we want the behavior to be in this situation?
Currently, I'm giving the imported splits an |
src/ui/RunEditor.tsx
Outdated
| @@ -1858,6 +1872,18 @@ export class RunEditor extends React.Component<Props, State> { | |||
| } | |||
|
|
|||
| private async downloadSplits<T>(apiRun: Run<T>, apiUri: string) { | |||
| // TODO: Determine whether there are any unsaved changes in the Run Editor | |||
There was a problem hiding this comment.
I think being able to close the Run Editor without disposing it would allow us to handle this?
src/ui/TimerView.tsx
Outdated
| @@ -246,7 +246,7 @@ export class TimerView extends React.Component<Props, State> { | |||
| toast.error(e); | |||
| }; | |||
| this.connection.onmessage = (e) => { | |||
| // FIXME: Clone the Shared Timer. This assumes that `this` is always | |||
| // TODO: Clone the Shared Timer. This assumes that `this` is always | |||
There was a problem hiding this comment.
I started using FIXME as long term things that need to be / should be fixed and TODOs being temporary self reminders to do something as part of the ongoing PR. So unless you plan to fix this as part of this PR, this should stay a FIXME.
There was a problem hiding this comment.
In that case, can we rename our existing TODOs to FIXMEs instead?
|
If you edit splits that aren't open and download splits, it currently replaces the splits in your timer, despite you never "touching your timer". I don't think that's what we want. Honestly, I feel like we long term probably don't even want to have the leaderboard in the run editor. But that's outside the scope of this PR. |
|
@CryZe Should we always persist the splits from the leaderboard to the database if you click "OK"? That seems like the least confusing user experience based on what you said. |
|
Yeah I think that may be fine? I'm honestly not sure. |
|
Also, I agree that the leaderboard shouldn't be in the Run Editor. |
wooferzfg
force-pushed
the
leaderboard-splits
branch
from
d110af3
to
062860e
Compare
wooferzfg
force-pushed
the
leaderboard-splits
branch
2 times, most recently
from
9d80db8
to
1dee3a6
Compare
wooferzfg
added
bug
wooferzfg
force-pushed
the
leaderboard-splits
branch
3 times, most recently
from
a4cac16
to
4545bb6
Compare
wooferzfg
force-pushed
the
leaderboard-splits
branch
3 times, most recently
from
fe93f26
to
9473772
Compare
wooferzfg
force-pushed
the
leaderboard-splits
branch
from
9473772
to
33d15a0
Compare
|
I can re-review this tomorrow maybe. |
|
@CryZe Could you take a look at the way I'm handling the run editor in this PR? Not sure if I'm disposing it properly everywhere |
persistChanges. This determines whether changes made in the Run Editor should immediately be saved to the database or not. We were previously using the splits key to determine this, but we now need that in order to switch between runs while the Run Editor is open.Fix #362