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

Latest Updates landing page #478

Merged
merged 36 commits into from
Sep 10, 2016

Conversation

xtine
Copy link
Contributor

@xtine xtine commented Sep 4, 2016

Latest Updates

This sets up a simple view to show Press Releases, News Digests, and Records on one landing page for #474.

@emileighoutlaw
Copy link
Contributor

I pushed some capitalization changes here! Very small! Not trying to be a grammar bully :hurtrealbad: ❤️

@codecov-io
Copy link

codecov-io commented Sep 6, 2016

Current coverage is 86.02% (diff: 49.24%)

Merging #478 into develop will decrease coverage by 4.09%

@@            develop       #478   diff @@
==========================================
  Files            50         55     +5   
  Lines          1224       1345   +121   
  Methods          42         42          
  Messages          0          0          
  Branches         54         69    +15   
==========================================
+ Hits           1103       1157    +54   
- Misses          117        183    +66   
- Partials          4          5     +1   

Powered by Codecov. Last update 02d56e0...b329576

Noah Manger added 2 commits September 6, 2016 16:18
- Query update types and categories based on request params
- Combine all update types into a single list
- Add pagination
- Add basic navigation and categories
@noahmanger
Copy link

Nice! This is a great start. I can take it from here.

@noahmanger
Copy link

I hit a stride, so I just kept running with this. This PR now includes the new press landing page, complete with press release feed:

image

And all the makings of a filterable feed of all the pages (placeholder design for now):

image

@noahmanger noahmanger changed the title [WIP] Latest Updates landing page Latest Updates landing page Sep 8, 2016
@noahmanger
Copy link

Ok, I have the visual designs from https://github.com/18F/fec-cms/issues/454 implemented, with a couple suggested variations purely for the sake of getting an MVP version out the door.

The feed of items is as designed (minus the "read more" links in the blurbs, which would take more work, but can happen later).
image

  • Press release and digest type updates say "From: Press office" and FEC Record type updates are "From: Information division"
  • Links go to a filtered view for that update type
  • Titles link to pages
  • All posts are sorted most recent first

Where I'm diverging from the mocks is in the filters:
image

  • For the sake of simplicity, I wanted to do this with as little javascript as possible. This means two things:
    • Using native form inputs rather than our special dropdowns (which would need to be updated to support slightly different behavior)
    • Relying on page reloads to show updated data, rather than doing an asynchronous request to get results, which would take more time to set up. So the page reloads after every form change.
  • Because regular html select elements can't support multiple values, I use checkboxes here
  • By default, all checkboxes are empty and the "subjects" select is disabled
  • Checking "press releases" reloads the page, showing only press releases and enables a "press release subjects checkbox:
    image
  • Unselecting it disables the checkbox again
  • The same works for "FEC Record":
    image
  • If both "FEC Record" and "Press releases" are checked, then two selects are shown, one for each type:
    image
  • Filtering by year works as expected

I'd like to start getting a code review on this now that it just works because it's pretty big. We can discuss interaction specifics here though. I'm totally open to changes, but would like to try to minimize dev time.

@onezerojeremy
Copy link

Probably I'm reading this wrong, and I don't reallllly know what i'm talking about here, but I don't think we need to support selection of multiple values in the content type selection. Users can pick:

  • for media pros : reloads page with a new query string that has &type=weekly_digests,news_releases
    • weekly digest: reloads page with a new query string that has &type=weekly_digests
    • news releases : reloads page with a new query string that has &type=news_releases
  • for committees: reloads the page with a new query string that has &type=record
    • the record: reloads the page with a new query string that has &type=record

Digest is part of the name, so I moved it to caps
Noah Manger and others added 3 commits September 7, 2016 18:35
No more lorem ipsum! Yay!
I'm very fond of this subversive colon. If others disagree, we can discuss! :)
@emileighoutlaw
Copy link
Contributor

emileighoutlaw commented Sep 8, 2016

This is looking great!

I made some style changes and added some content, so you don't have to work in lorem ipsum on the Latest updates page.

There's still some content needed for the Press landing page, but I want to work with Judy to develop it— she's a fantastic writer, and I want her to feel excited about this. I'll ask Amy what the best path forward is tomorrow a.m.

I added in a stray "the". Whoopsies
@jenniferthibault
Copy link
Contributor

jenniferthibault commented Sep 8, 2016

+1 to please no checkboxes.

@onezerojeremy / @nickykrause Are we intending to have validation happening with these dropdown & year entry fields?

cc @nickykrause just for visibility (I only found this pull request by chance when @emileighoutlaw mentioned she put a content note in here, and there are interaction things being discussed.)

@noahmanger
Copy link

So, not doing checkboxes means that only one option can be selected at time. But I did figure out a way to have options (like "for media professionals") that the server can use to render multiple update types:

image

So selecting "for media professionals" returns a list of press releases and digests, and shows the press release category. Selecting "For committees" or "FEC Record" does the same thing.

@onezerojeremy
Copy link

that looks great to me! Thanks for the fancy footwork, @noahmanger !

@noahmanger
Copy link

@jenniferthibault ? @nickykrause ? what do you think?

@nickykrause
Copy link

@noahmanger

This looks good to me, although I do wonder whether or not users will realize that "For media professionals:" is selectable (rather than just a label). But, either way, I do like what you've done with that option to enable users to select both press releases and digests, if they desire.

Seems good for now, at least, I think!

@jenniferthibault
Copy link
Contributor

+1 to what the smart people above said 😺

@@ -172,6 +170,10 @@ class RecordPage(ContentPage):
def content_section(self):
return ''

@property
def get_update_type(self):
return 'FEC Record'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be referring to the update_types OrderedDict defined above instead?

@noahmanger noahmanger changed the base branch from develop to release/public-beta-20160914 September 10, 2016 00:01
@noahmanger
Copy link

Updated this to be against the release branch.

@ccostino
Copy link
Contributor

Awesome, thanks @noahmanger!

@ccostino ccostino merged commit 3fb54bc into release/public-beta-20160914 Sep 10, 2016
@ccostino ccostino deleted the feature/latest_updates branch September 10, 2016 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants