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

Hide map for /yourschool views from outside the US #19933

Merged
merged 1 commit into from Jan 11, 2018

Conversation

drewsamnick
Copy link
Contributor

To make /yourschool more friendly to people not in the US we are going to hide the map if the IP is not determined to be from the US.

Before:
screen shot 2018-01-11 at 8 30 39 am

After (if not US):
screen shot 2018-01-11 at 8 29 31 am

@drewsamnick
Copy link
Contributor Author

Copy link
Member

@breville breville left a comment

Choose a reason for hiding this comment

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

Assuming /yourschool gets proper locations via a geocoder at request time (sorry, I forget.. some of our pages do it asynchronously via an ajax call) then this seems good.

@drewsamnick
Copy link
Contributor Author

@breville I was able to confirm that I get 'us' when I view the page. I wasn't sure how I can test for other countries. Do you have any ideas?

@Erin007
Copy link
Contributor

Erin007 commented Jan 11, 2018

It seems a little strange that if we know they aren't in the US to hide the map, we still default the country to US in the dropdown?

{i18n.yourSchoolMapDesc()}
If you are located in the US, please <a href="#form">fill out the form below</a>.
If you are outside the US, <a href="/learn/local">add your school here</a>.
</h3>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this string still make sense if we've made the determination via IP whether they are in the US or not already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this as is because the IP-based country determination isn't 100% accurate.

@drewsamnick
Copy link
Contributor Author

Setting the default country is a bit more involved since the dropdown component expects a string with the name of the country vs. the two letter country code. It also requires a bit of new plumbing to get that value from the HAML component to the javascript to the React. We were looking for a minimal change to make the page less US-centric.

That said, I'd like to change the default country choice if I can make the time to work on it.

@drewsamnick drewsamnick merged commit f4f0e0a into staging Jan 11, 2018
@drewsamnick drewsamnick deleted the census-hide-map-outside-us branch January 11, 2018 22:07
breville added a commit that referenced this pull request Apr 5, 2018
This undoes the work to hide the map for non-US users in #19933, though leaves the map hiding functionality available in case we need it for something else.
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.

None yet

3 participants