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

#1635 Add a special county agency to show all county freqs without an agency #1724

Merged
merged 2 commits into from Nov 11, 2023

Conversation

EricTendian
Copy link
Contributor

@EricTendian EricTendian commented Nov 11, 2023

Closes #1635

This solves the problem of frequencies tied to the main county page in RadioReference not showing up in the list of frequencies available to import. I'm doing this via a new subclass called CountyAgency which acts like an agency, but gets the frequency categories from the county instead of an agency. It will always show up first in the UI, like so:

image

For counties with a very large number of categories (and likely subcategories), it will default to showing the first category in the list, instead of trying to show all the frequencies at once (which I've found to be extremely slow):

image

On other smaller counties though, it will have the normal (All) selection.

@GTR8000
Copy link

GTR8000 commented Nov 11, 2023

I just tested this with the latest source code of beta5 and found a glitch, otherwise it seems to be working well.

  • Navigate to County A, then navigate to County B, then return to County A. X County (All) gets duplicated each time you bounce back and forth between counties (an additional entry for each is added at the top of the list).

@EricTendian
Copy link
Contributor Author

@GTR8000 Thanks for testing it! You're right, it looks like I forgot that Java will pass the original object along, so it kept adding the CountyAgency to the original list of agencies in the API. Now I'm effectively copying the list and don't see the same issue anymore.

@GTR8000
Copy link

GTR8000 commented Nov 11, 2023

Yup, looks good. @DSheirer tested with very latest Beta 5 source code, working well, recommend merging ASAP

@DSheirer DSheirer self-requested a review November 11, 2023 09:21
Copy link
Owner

@DSheirer DSheirer left a comment

Choose a reason for hiding this comment

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

Great submission, thank you!

@DSheirer DSheirer merged commit ae31c41 into DSheirer:master Nov 11, 2023
@EricTendian EricTendian deleted the county-freqs-without-agency branch November 11, 2023 14:52
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.

RadioReference import doesn't show all county systems
3 participants