Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

styling the no results message #628

Merged
merged 1 commit into from
Sep 11, 2015
Merged

Conversation

noahmanger
Copy link
Contributor

Styles the message that shows up if the zip code isn't found. @emileighoutlaw how doest this language work for you? Made a slight adjustment from @LindsayYoung 's original.

screen shot 2015-09-10 at 4 52 13 pm

@jmcarp
Copy link
Contributor

jmcarp commented Sep 11, 2015

The language looks good, but I realized there's a minor issue here: it's possible (although pretty unlikely) for users to see this message even when searching by state and district. Occasionally, we run into missing data (e.g., Virgin Islands 1997-98) and show no results. In those cases, saying we couldn't find results for the specified zip code doesn't make sense.

I'm going to condition the warning text on search state: if the user specified a zip code, we'll warn that we couldn't find results "for this ZIP code"; else, that we couldn't find results "for this location". If we want to update that language or behavior, we can file another patch.

@noahmanger: I'm also going to fix the unit tests on merging. @LindsayYoung added a JS unit test asserting that the expected warning text was added when no results were found, and that test broke when the text changed.

@jmcarp jmcarp merged commit b6957d1 into develop Sep 11, 2015
@jmcarp jmcarp removed the plz-review label Sep 11, 2015
@noahmanger
Copy link
Contributor Author

Sounds good to me. Thanks.

On Fri, Sep 11, 2015 at 6:52 AM, Joshua Carp [email protected]
wrote:

Merged #628 #628.


Reply to this email directly or view it on GitHub
#628 (comment).

Noah Manger
18F http://18f.gsa.gov | Work: 415-696-6146

@emileighoutlaw
Copy link
Contributor

I missed the boat, but wanted to pop on and say anyway that this language has my thumbs up.

@jmcarp jmcarp deleted the feature/no-results-message branch October 2, 2015 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants