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

feat: show freshers events on get involved page #375

Merged
merged 3 commits into from
Sep 15, 2024

Conversation

alyxbb
Copy link
Contributor

@alyxbb alyxbb commented Sep 15, 2024

No description provided.

@ColinRoitt
Copy link
Contributor

Any reason for OSM rather than google maps out of interest?

@Keeeeeeeen
Copy link
Contributor

Hi, we probably shouldn't be hardcoding information into the website that will be only useful for a week max, would it not make more sense to add a "public" or similar flag to events on the calendar that already exists on myradio, and then pull the public events to the website? Or atleast make it toggle able in the config so we don't have to push a new release just to disable the Freshers events?

@ColinRoitt
Copy link
Contributor

Hi, we probably shouldn't be hardcoding information into the website that will be only useful for a week max, would it not make more sense to add a "public" or similar flag to events on the calendar that already exists on myradio, and then pull the public events to the website? Or atleast make it toggle able in the config so we don't have to push a new release just to disable the Freshers events?

I'd support not doing that purely for speed as we want this up fast I assume

@markspolakovs
Copy link
Member

markspolakovs commented Sep 15, 2024

Any reason for OSM rather than google maps out of interest?

Seconded; it makes sense for an embedded map where you don't really care how it looks as long as you can see it, but for something like this I'd say it'd be better to use something more commonly used (and also with directions support, so I can click the link on my phone and get directions to the place).

@hitime1234
Copy link
Contributor

hitime1234 commented Sep 15, 2024

I agree hardcoding it like this might bit of a pain when it passes freshers week.
Prehaps make it togglable in config.toml so we don't have do another pr to remove it?
see #363 for example

@alyxbb
Copy link
Contributor Author

alyxbb commented Sep 15, 2024

Ive changed the links to gmaps.
I would like to make it configurable, but that will take quite a lot of time to make it fully configurable. I would like to get it released today or tomorrow as freshers are already looking at the website.
I plan to revert the commit after freshers and then work on making it configurable when i get the time to

@alyxbb alyxbb merged commit d185da1 into development Sep 15, 2024
2 checks passed
@alyxbb alyxbb deleted the alyx-2024-get-involved branch September 15, 2024 20:41
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.

6 participants