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

Make venue_info a (configurable) select #109

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ghalse
Copy link
Contributor

@ghalse ghalse commented Jan 17, 2025

Expecting users (even administrative ones) to look up a venue info code on a web page and then enter it into a blank-allowed field means we rarely get venue info completed.

This converts the venue_info fields into drop-town selects, based on the normative categories from IEEE Std 802.11-2020 table 9-65 (the latest publicly accessible version). It also changes the default value from blank to 0,0 (unspecified).

The VENUE_INFO data is placed in djnro/settings.py to make it easier for DjNRO admins to expand or localise the list if they need to. However, we probably shouldn't be encouraging people to change the IEEE values (or even translate them?). There's a note to that effect in settings.py, and the config is deliberately omitted from local_settings.py.

@ghalse
Copy link
Contributor Author

ghalse commented Jan 17, 2025

This change reverts @vladimir-mencl-eresearch's addition in PR #98

@vladimir-mencl-eresearch
Copy link
Collaborator

Hi @ghalse ,

Thanks - this is a great improvement over having to look up and enter numeric value. Not sorry at all to see my link to GEANT wiki go.

I've just deployed this branch locally and discussed it with my colleagues. Got back feedback suggesting to also show the numerical codes as part of the name as well - like:

2 : Business
2,7 : Company Office
2,8 : Research Facility

Your thoughts?

I also noticed on the Institution details page (/manage/institutions), only shows the numerical code is shown - so here, it might be helpful to also show the descriptions.

Would that be easy to add?

Cheers,
Vlad

@ghalse
Copy link
Contributor Author

ghalse commented Feb 1, 2025

Got back feedback suggesting to also show the numerical codes as part of the name as well - like:

Because the drop-down is rendered using a Django choice and the current version of Django doesn't support overriding the built-in templates, I don't think adding this in is possible without changing the descriptions themselves. That means I can't make it a configurable option.

I'm inclined not to include them, for two reasons:

  • none of the other UIs I've seen implementing this sort of drop-down expose the actual numeric values to users; and
  • I'd deliberately set the descriptions to use the exact text from IEEE Std 802.11-2020 without trying to interpret them in any way. Adding the numeric values into them would deviate from that.

However, that's exactly why I did it as a configurable option. If you want them to appear in your version, override the VENUE_INFO structure in your local_settings.py to include them.

I also noticed on the Institution details page (/manage/institutions), only shows the numerical code is shown - so here, it might be helpful to also show the descriptions.

There was a reason I didn't do this, but that might date back to an earlier attempt where I had the descriptions in the view. I'll look at how easy it is.

@ghalse ghalse force-pushed the patch-venue-info-select branch from 8e76aa9 to 9671b7e Compare February 1, 2025 15:25
@ghalse
Copy link
Contributor Author

ghalse commented Feb 1, 2025

I also noticed on the Institution details page (/manage/institutions), only shows the numerical code is shown - so here, it might be helpful to also show the descriptions.

There was a reason I didn't do this, but that might date back to an earlier attempt where I had the descriptions in the view. I'll look at how easy it is.

Turns out it's trivial :-)

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.

2 participants