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

#106- Implement Officials centric view of matches #107

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

philipohagan
Copy link
Contributor

Thank you for contributing to this application!

Closes #106

Proposed Changes

Explain the changes you've made with this PR

  • Implement ics files on a per official (per fetcher) basis
  • Add appropriate front end changes to allow the files to be accessed

Checklist

  • I have read the contribution document.
  • I have added test cases to cover my changes (if applicable).
  • All new and existing test cases have passed.
  • My code adheres to the coding style of this project.

@philipohagan philipohagan marked this pull request as ready for review December 3, 2024 18:45
@philipohagan
Copy link
Contributor Author

@martijnvankekem - won't let me assign you this for some reason

@martijnvankekem
Copy link
Contributor

I like the idea, but I have some notes:

  • Is there a reason why the official-specific calendars are only there for the EH fetcher, and not for the other TMS sources?
  • The input for official selection is very prominent, while I think there's gonna be only a small amount of people actually using this feature. I'd like to see the input a bit more subtle. Maybe a small select button that pops out when clicked?
  • [Bug]: the official warning doesn't disappear when selecting an official and then changing sources.

docs/app.js Outdated Show resolved Hide resolved
src/Utils/ICSCreator.ts Show resolved Hide resolved
src/Utils/ICSCreator.ts Show resolved Hide resolved
@philipohagan
Copy link
Contributor Author

philipohagan commented Jan 3, 2025

Thank you - appreciate it

  • Is there a reason why the official-specific calendars are only there for the EH fetcher, and not for the other TMS sources?

Good spot- the source was hardcoded in the front end, I've changed this to use the 'origin' dynamically

  • [Bug]: the official warning doesn't disappear when selecting an official and then changing sources.

Thanks- resolved.

  • The input for official selection is very prominent, while I think there's gonna be only a small amount of people actually using this feature. I'd like to see the input a bit more subtle. Maybe a small select button that pops out when clicked?

Going to work on this now.

@philipohagan
Copy link
Contributor Author

Ready for review

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.

[FEATURE REQUEST]: Add official centric ICS files
2 participants