CodeForPhilly / clean-and-green-philly

Dashboard to target Philly vacant properties for anti-gun violence interventions
https://www.cleanandgreenphilly.org/
MIT License
32 stars 58 forks source link

Feature: Save properties #415

Closed Moylena closed 3 months ago

Moylena commented 6 months ago

Currently there is no way to save a property in the property detail page

Please add the icon in the top right hand of the property detail card, on the same line as the "back" link. Include the default and active state as a part of this ticket

Image

Image

When user clicks this new icon, all properties with this "save" flag will appear. It's another filtering mechanism.


See Figma file for details: https://www.figma.com/proto/NAFkgq34abW6uJ0R7PW24T/Prototype---Clean-%26-Green-Philly?type=design&node-id=2592-30687&t=sdhkPVHItWMM3Xnn-0&scaling=min-zoom&page-id=187%3A12602&starting-point-node-id=2592%3A30019

paulhchoi commented 5 months ago

@thansidwell @brandonfcohen1 this may be something we'd want to discuss further, as this is more involved then just creating a button — as there is no currently concept of a 'user', I believe the most we can do to "save a location" is to add the location_id (or similar) to the user's browser's localStorage, but that may lead to unexpected UX (a user browsing on DuckDuckGo or Firefox's temporaryContainers would have their localStorage deleted every session, or a user clearing their browsers cache/cookies unexpectedly deletes their saved selections, etc.). The more robust option would be to create user logins and store the selections to a database, but that would require cost/platform choice discussions.

brandonfcohen1 commented 5 months ago

Yeah, @nlebovits and I had talked about this from the start- to just use localStorage. I think that's fine as a first step honestly, and anyone using DuckDuckGo/incognito likely knows that would happen anyway

CodeWritingCow commented 5 months ago

Hello, I'd like to tackle this ticket. Can someone assign this to me? Thank you!

Moylena commented 5 months ago

@CodeWritingCow Hi is this complete?

CodeWritingCow commented 5 months ago

@CodeWritingCow Hi is this complete?

Hi @Moylena I'm working on this ticket. I've added the "Save" button to the property detail card UI as shown in Figma. I plan to submit a PR this week. However, I do have a couple questions about this requirement:

"When user clicks this new icon, all properties with this "save" flag will appear. It's another filtering mechanism."

As a user, how would I save properties in the dashboard? And how do I then filter them?

Figma shows each property's detail card UI as having a "Save" button in its upper right corner. Originally, I assumed that when users click the button, that saves or "unsaves" the property to their Web browser's localStorage. However, the requirement describes the button as a filtering mechanism to show saved properties (and hide "unsaved" ones).

Am I misunderstanding the button's purpose?

Moylena commented 5 months ago

@CodeWritingCow did you review the previous comments? I think they outline the technical expectations

CodeWritingCow commented 5 months ago

@Moylena Yes, I did review the comments about saving a property's location ID to the browser'slocalStorage. I do understand and can implement that. However, before I submit a PR, I'd like to verify with you the Save button's expected behavior. ... When a user clicks the Save button, will that add a property to a list of saved properties, and also filter out properties that are not saved?

(Yesterday's standup would have been a great opportunity to discuss this. However, I thought of it only today as I went over my work in progress.)

Moylena commented 5 months ago

Got it. @thansidwell can you confirm please? I understood it to be a way to mark a given property and later have the ability to filter by "saved".

thansidwell commented 5 months ago

Hey @CodeWritingCow @Moylena @brandonfcohen1 @paulhchoi

Here's an explanation of the UX I worked out. Sorry I didn't add this in sooner.

Here's a video walk through.

https://github.com/CodeForPhilly/vacant-lots-proj/assets/1965986/26030404-d5c9-48cd-be99-af5c9f44bf8e

CodeWritingCow commented 5 months ago

Hi @thansidwell and @Moylena, thanks for following up on my question and the thorough walkthrough! The video and the step-by-step UX explanation are really helpful. I'll update my work accordingly and then submit a PR this week. Thanks again!

CodeWritingCow commented 4 months ago

@all-contributors please add @CodeWritingCow for code

allcontributors[bot] commented 4 months ago

@CodeWritingCow

This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected token : in JSON at position 3695

Moylena commented 3 months ago

Confirmed live