Skip to content

Fixing "next match" feature#1498

Merged
varCepheid merged 30 commits intodevfrom
fixing-next-match
Feb 18, 2025
Merged

Fixing "next match" feature#1498
varCepheid merged 30 commits intodevfrom
fixing-next-match

Conversation

@varCepheid
Copy link
Collaborator

@varCepheid varCepheid commented Apr 8, 2022

closes #1493
At the top of the event and event-team pages, instead of one "next match," the page shows the three most relevant upcoming matches.

@varCepheid varCepheid requested a review from calebeby April 8, 2022 00:33
@varCepheid varCepheid self-assigned this Apr 8, 2022
@calebeby
Copy link
Member

calebeby commented Apr 8, 2022

Hey @varCepheid, usually the scheduled match time and the predicted match time are wrong by 5-10 minutes. Do you think that instead of this solution we could show "upcoming matches" as 2-3 matches?

… before and three after the current time; in fixing some aestethic issues a couple more popped up, but I don't know exactly how to fix those (and they're a bit less noticeable)
@ev118 ev118 self-assigned this Apr 11, 2022
@ev118
Copy link
Contributor

ev118 commented Apr 12, 2022

I think the problem has been solved, though a bit of tweaking may be useful so that it looks neat (the Upcoming Matches header is off-center, even though it says justify-self: center; and the search bar sits right on top of Qual 1).

@varCepheid varCepheid removed their assignment Apr 12, 2022
@calebeby
Copy link
Member

Hey @ev118, thanks for updating this! I think there are several opportunities to simplify the approach here, would you be open to scheduling a meeting this weekend to work through that with me?

@varCepheid varCepheid added bug Addresses something that isn't working. needs work Changes have been suggested and need to be implemented. labels May 9, 2022
@varCepheid varCepheid marked this pull request as draft June 6, 2022 04:41
@calebeby calebeby added the needs work Changes have been suggested and need to be implemented. label Nov 12, 2023
@calebeby
Copy link
Member

@varCepheid I also noticed that the issue #1493 is kind of vague and doesn't mention whether it is talking about the next match feature on the team@event page or on the event page. This PR only fixes the team@event page, so maybe we should leave that issue open when this is merged and open another PR for the event page.

removed "No Upcoming Matches" on event-team page
added space below header
changed getUpcomingMatches to new series of filters
@varCepheid
Copy link
Collaborator Author

I agree that the issue is vague. I added a description on the PR, which is hopefully more clear. And the changes here do affect the event page; if it wasn't doing anything different, we should definitely fix that here.

@varCepheid varCepheid removed the needs work Changes have been suggested and need to be implemented. label Nov 15, 2023
@varCepheid varCepheid self-assigned this Nov 15, 2023
@varCepheid varCepheid requested a review from calebeby November 15, 2023 01:11
@varCepheid
Copy link
Collaborator Author

When I merged from dev, it brought a bunch of changes to package-lock.json. I tried to deal with all of them, but the file is so large that it was lagging a lot, and I think I made a mistake somewhere. Should I try to revert the commit?

@calebeby
Copy link
Member

@varCepheid sorry for not getting back to you sooner. You shouldn't ever need to manually merge a package-lock.json file. You can always just checkout the package-lock.json from either side of the merge, and then run npm i to make npm fix any issues or discrepancies from the package.json file. In this case, there are no changes to package.json so we don't even need to do that. Since the changes to package-lock.json aren't relevant to this PR, I went ahead and reverted them. (git checkout origin/dev -- package-lock.json is the easiest way to do this for a single file without necessarily reverting an entire commit, it just means "set this file to as it was on _ branch").

@varCepheid
Copy link
Collaborator Author

Is there anything else that you need to review, or is this ready to push?

calebeby
calebeby previously approved these changes Dec 14, 2024
Copy link
Member

@calebeby calebeby left a comment

Choose a reason for hiding this comment

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

Looks good just left some comments about the eslint-disable comments. Feel free to merge after addressing those.

I've not done recent testing on this but I trust that you have. Since there are not any TBA events happening right now (other than bunnybots which doesn't have a schedule) you can probably test this by replacing the current time in the code with a fake time from the past to see if it behaves correctly.

@varCepheid varCepheid added this pull request to the merge queue Dec 14, 2024
@calebeby calebeby removed this pull request from the merge queue due to a manual request Dec 14, 2024
@varCepheid varCepheid added this pull request to the merge queue Feb 18, 2025
Merged via the queue into dev with commit 8ee6f47 Feb 18, 2025
12 checks passed
@varCepheid varCepheid deleted the fixing-next-match branch February 18, 2025 04:29
github-merge-queue bot pushed a commit that referenced this pull request Apr 14, 2025
* updated comment-card max width

trying to fix issue 1480

* changed "next match" to be based on time

Scores don't upload quickly, so this appears to fix the issue. Also ran lint.

* the problem has been solved, it now shows five matches, in theory two before and three after the current time; in fixing some aestethic issues a couple more popped up, but I don't know exactly how to fix those (and they're a bit less noticeable)

* [WIP] change implementation and fix some CSS bugs

* [WIP] formatting changes on event-team pages. I couldn't test on any real events so it needs checking. --Elijah

* testing with manual date/time

* same

* testing changes

* same

* testing is hard

* removed score check for upcoming matches function

* changed time span for upcoming-matches

* changed times again

* more testing for upcoming matches

* reverted testing changes

* removed comments

* requested edits

removed "No Upcoming Matches" on event-team page
added space below header
changed getUpcomingMatches to new series of filters

* Revert changes to `package-lock.json`

* undo unrelated changes

---------

Co-authored-by: ev118 <92764232+ev118@users.noreply.github.com>
Co-authored-by: Caleb Eby <caleb.eby01@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Addresses something that isn't working.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Next match" feature isn't always accurate

3 participants