CouncilDataProject / seattle_v2

Website for interacting with Seattle's instance of CDP
https://councildataproject.github.io/seattle
BSD 3-Clause "New" or "Revised" License
5 stars 6 forks source link

show minutes file in details tab #31

Closed tohuynh closed 4 years ago

tohuynh commented 5 years ago

Issue 12 Show files as link for minutes item that have them

evamaxfield commented 5 years ago

Thanks for your PR!

Relevant issues: Resolves #12

Link to staging that shows functionality: https://jacksonmaxfield.github.io/cdp-frontend-dev/#/events/108a464d-3852-44b8-b0ac-987b6ed5012e

Notes to other reviewers: Some of the minutes items names look odd, e.g. IRC 206 May 13, 2019. The matter name is the IRC 206 and the May 13, 2019 is the minutes item name. This is not caused by this front end development but rather what I will call "corrupted" data on the database. I have had a fix in progress on the back end for a very long time so hopefully this will be fixed soon. Ignore for now though. Relevant back end issue

tohuynh commented 5 years ago

From your example, minutes_item.matter is IRC 206, and minutes_item.name is May 13, 2019. Sometimes matter is null, so I display name only. In cases where there are both matter and name, and they are different, I display both. Is that alright?

evamaxfield commented 5 years ago

From your example, minutes_item.matter is IRC 206, and minutes_item.name is May 13, 2019. Sometimes matter is null, so I display name only. In cases where there are both matter and name, and they are different, I display both. Is that alright?

I think what you have is perfect and if already future proofed from the backend updated. minutes_item.matter should always be the bolded text and the minutes_item.name should always be the "description" / "subtitle" text imo. This is because usually the minutes item name will be things like "A RESOLUTION on x..." So yes, totally fine, and preferred!

evamaxfield commented 5 years ago

Could you alphabetize the supporting document links?

tohuynh commented 5 years ago

Yes, by using minutes_item_file.name

evamaxfield commented 4 years ago

Thanks for updating to alphabetize the minutes files! This looks good and I am going to merge!