coregis / cbf-programs-map-2023

Rebranding and other updates to the Raise Your Hand Texas Programs Map
GNU General Public License v3.0
0 stars 0 forks source link

Switch to referring to ESCs by number #32

Closed eldang closed 10 months ago

eldang commented 10 months ago

Resolves #31

Please test this one before merging! As you'll see from the change list and commits, it wasn't quite as simple a switch as I'd expected, which has me worried I may still have missed something.

coregis commented 10 months ago

Resolves #31

Please test this one before merging! As you'll see from the change list and commits, it wasn't quite as simple a switch as I'd expected, which has me worried I may still have missed something.

This is working well! However I think there are a couple things that need attention before we call it good:

It looks like the drop-down for districts is bottoming out at Baird ISD; is it possible to retain the complete list so if the user wants to scroll (for days) they still can?

image

It looks like the Education Service Centers dropdown is not getting the Work Sans styling from CSS:

image

nein09 commented 10 months ago

It looks like the drop-down for districts is bottoming out at Baird ISD; is it possible to retain the complete list so if the user wants to scroll (for days) they still can?

Yes! That's a TomSelect configuration option, apparently. Give it a try now.

nein09 commented 10 months ago

ugh, I just noticed that the 'view whole state' button doesn't reset the school district select and I don't have time to fix it today.

eldang commented 10 months ago

I'll take a look at the CSS part later today. There's probably either a class I should have added to the ESCs dropdown, or a CSS selector I need to add its id to.

eldang commented 10 months ago

I think I've sorted out both the styling and the resetting of the ISDs dropdown.

nein09 commented 10 months ago

I think I've sorted out both the styling and the resetting of the ISDs dropdown.

You could also try setValue('-108,25,-88,37,0', true); if you want it to reset to the default value of "School District".

eldang commented 10 months ago

You could also try setValue('-108,25,-88,37,0', true); if you want it to reset to the default value of "School District".

Is there a meaningful difference in behaviour?

nein09 commented 10 months ago

You could also try setValue('-108,25,-88,37,0', true); if you want it to reset to the default value of "School District".

Is there a meaningful difference in behaviour?

It explicitly selects 'School Districts' as an option instead of clearing the selection, so it looks more like the starting state of the page because the control isn't greyed out. Aside from that, no, there is not.

eldang commented 10 months ago

It explicitly selects 'School Districts' as an option instead of clearing the selection, so it looks more like the starting state of the page because the control isn't greyed out. Aside from that, no, there is not.

Ah, thank you, I'd missed that detail. My gut feeling is that it's actually worth leaving this as the clear() method to not have an additional place where a zoom target is hand coded. But it's really @coregis's call - what do you think?

coregis commented 10 months ago

It explicitly selects 'School Districts' as an option instead of clearing the selection, so it looks more like the starting state of the page because the control isn't greyed out. Aside from that, no, there is not.

Ah, thank you, I'd missed that detail. My gut feeling is that it's actually worth leaving this as the clear() method to not have an additional place where a zoom target is hand coded. But it's really @coregis's call - what do you think?

I agree with leaving this as the 'clear()' method, it is working well!