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

Feature/432 search #165

Merged
merged 30 commits into from
May 29, 2015
Merged

Feature/432 search #165

merged 30 commits into from
May 29, 2015

Conversation

msecret
Copy link
Contributor

@msecret msecret commented May 12, 2015

Work in progress

Check commit message for more info

Marco Segreto added 2 commits May 12, 2015 13:01
This uses a fake def on the backend to get a candidate/committee with
one opposite type nested in it.

TODO:
- Rename nested type to "primary"
- Finish template side
I'm using a new html class prefix for selenium tests, "tst". I thought
about this concept at my last job, am not sure about it and how useful
it will be. Essentially, I just don't want tests breaking because of
hooks failing from the HTML. I'm hoping seeing a class like this as a
dev will ensure you look at the tests when you change the hook.
Marco Segreto and others added 10 commits May 13, 2015 12:58
TODO:
- Style green bar
- Merge template so it can be used for committees
- Figure out time periods
TODO:
- Style green box.
- Logic for the timelines and currently active.
- Responsive check.
Fixed up a few other things as well.
I tried to abstract away the module a bit to allow for resuability.
TODO:
- The search filter interface
- Switch out the side panels for committees page.
@noahmanger
Copy link
Contributor

Looking good. Here's some things though (on a candidate search for obama):

  • Remove the "View all" button and "Candidate results" header from the top.
  • The associated committees aren't right. They should be the Principal Campaign Committee for each. There's a bunch that aren't even authorized
  • Let's remove the "most recent report" link
  • Replace the little arrows with »
  • For Senate and Presidential races, either hide "District" or replace it with "Statewide" / "Nationwide"
  • We can't sort by year, right? If not, can we at least sort to have Presidential candidates higher up?
  • The "View all current candidates" link goes to "/committees"
  • There's also some copy edits I know from the last round of feedback with FEC but I can make those before we merge.

@noahmanger
Copy link
Contributor

Oh, and also the searchbar in the header needs the type dropdown.

@noahmanger
Copy link
Contributor

Also, is there currently a limit on the search results? I'd like to try it with no limit.

@msecret
Copy link
Contributor Author

msecret commented May 18, 2015

@jmcarp can you look into Noah's comment "The associated committees aren't right. They should be the Principal Campaign Committee for each. There's a bunch that aren't even authorized"? I'm currently using the new search API route for the nested committee. Was that change not pushed to all envs?

@jmcarp
Copy link
Contributor

jmcarp commented May 18, 2015

@msecret checking that out now. Do you have an example of a committee that's wrong?

@noahmanger
Copy link
Contributor

@msecret have you had a chance to fix the things I noted above? Doesn't look like it yet or maybe I'm just out of date.

msecret added 4 commits May 27, 2015 11:22
Conflicts:
	__init__.py
	openfecwebapp/api_caller.py
	static/styles/_components/_search-bar.scss
	templates/search-results.html
This was done to ensure they UI matched the main search on the homepage
and so users could search by candidate/committee on every page.
The new search results page doesn't have an h2 with the query as the old
page did, so the tests were modified to just find a custom selector for
the results.

The "tst" html class prefix is being used specifically for tests.
@msecret
Copy link
Contributor Author

msecret commented May 27, 2015

  • Hide header search bar on home page
  • Change committee link to search
  • No limit on api call
  • Pass search term as q

msecret added 2 commits May 27, 2015 15:20
- Hide header search bar on home page, as main search bar is already
  there.
- Change committee link to search as thats what the text states.
- No limit on api call as its important for users to be able to see
everything when searching, as some reported not being able to find what
they were looking for.
- Pass search term as q so it works with existing setup and code.
- Removed test for header search on landing page as it was removed.
- Appending a message to top and bottom of search results when more
then 20 results (the current max for api) so the user knows they're not
seeing all results and can click a link to view more.
- Updating the top search bar to have the query input and the type
selected so the user knows the current search.
@msecret
Copy link
Contributor Author

msecret commented May 27, 2015

What the to many results UI looks like, @noahmanger you'll probably want to make modifications

snapshot2

@noahmanger
Copy link
Contributor

Perfect. Yeah I can work with that

On Wed, May 27, 2015 at 4:04 PM, Marco Segreto notifications@github.com
wrote:

What the to many results UI looks like, @noahmanger
https://github.com/noahmanger you'll probably want to make modifications

[image: snapshot2]
https://cloud.githubusercontent.com/assets/1701077/7849414/05730c24-048a-11e5-93f5-d0d29d672e9f.png


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

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

msecret pushed a commit that referenced this pull request May 29, 2015
@msecret msecret merged commit 906160f into fecgov:develop May 29, 2015
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.

None yet

4 participants