-
Notifications
You must be signed in to change notification settings - Fork 25
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
Tree Images #1: As a user, I want to be able to upload photos for a tree. #266
Comments
Hello, I would love to work on this issue. Please can elaborate more on this issue as I would like to be assigned to this issue |
Hi @Zhikky sounds great if you want to work on this!! In general, if there are issues that interest you, please assign yourself to them!! :) Currently there's images coming in here: https://github.com/waterthetrees/wtt_front/blob/main/client/src/pages/Tree/TreeImage.js They are just coming from wikimedia, not actual images of the street tree. We have not talked much about how this feature should work yet. Ideas: |
@zoobot The MVP of the tree upload feature would be like this: Uploading an imageOn the frontend, anyone can upload an image on any tree as long as they are signed in. The tree page will have an upload button that the user can click to upload an image, and this button will only appear when the user is signed in. On upload, a thumbnail is created along with the image, and the image and thumbnail are converted into uris and sent to the db. Whenever someone uploads an image, that image upload is added to the tree history and the history will link to the img. Luke and I initially thought that only users who adopted a tree should be able to upload images for them, but the WTT team argued that some users may simply want to upload images in their neighborhood without the additional responsibility of adopting trees. Luke and I also thought about only allowing uploads for certain categories of trees - not allowing uploads for dead or stump trees for example - but there may be use cases for uploading images of dead or stump trees too. Luke will handle the backend side of this, and they may create a separate write server which just handles image uploads, so the server which is handling reads doesn't get overloaded. Viewing imagesA tree's page will show all of its images in a carousel. The images could show the date uploaded so users have a clear idea of the timeline of the tree. Nice to HavesWe could add a social media share feature, so users could take tree selfies and share them. |
@ri0nardo Ok I moved the discussion back here from Slack. Given the figma design (which looks pretty cool!), it seems like we want to split the current tree slideout into tabs. The photos will be in an album in their own tab, and in the maintenance tab there will be a section to drag and drop or upload a file. Could the user also upload photos in the photos tab? It would be helpful if the Figma design had an image for the photos tab. |
@PGrad so currently there is a way to add photos to maintenance, but its not integrated yet. The idea is to eventually have section tabs for the tree info and for sub sections for photos. But I wouldn't worry about albums / sections for photos as that just a nice to have feature. So currently the design for the tree container is #433 . If you want to look into it, could help with how it works. |
@PGrad feel free to ask questions, upload screenshots of your progress here. Everyone is willing to answer any type of questions you may have. |
* chore: update to React 18 I want to use this react-spring-carousel component, which is dependent on React 18. I update react and react-dom. To do the update, I had to uninstall the deprecated @mui/styles library. * fix: update snapshots I don't think these snapshots have anything to do with the React update or removal of @mui/styles. The changes have to with CSS styles produced by Material UI components, which must be a side effect of a previous update to MUI which is not in our control. * chore: update npm packages I saw Dependabot was giving some errors about security vulnerabilities in outdated packages, so I updated everything (within its semver range) using `npm update`. * Revert "fix: update snapshots" This reverts commit 1c263a7. * fix: update Tree.test.js snapshot This snapshot is failing due to MUI changes, but it needs to be updated for CI to pass.
* fix: use createRoot() React 18 uses createRoot() now instead of ReactDOM.render(). I make the replace- ment to fix the error. * perf: use system fonts before load This fixes the Lighthouse error "Ensure text remains visible during webfont load". If the font hasn't loaded yet, you might get invisible text. `font-display: swap` fixes this by using the system font until the correct one has loaded. It also improves website load.
* fix: use createRoot() React 18 uses createRoot() now instead of ReactDOM.render(). I make the replace- ment to fix the error. * perf: use system fonts before load This fixes the Lighthouse error "Ensure text remains visible during webfont load". If the font hasn't loaded yet, you might get invisible text. `font-display: swap` fixes this by using the system font until the correct one has loaded. It also improves website load.
Just a status update. I added storybook to our repo so that I can build and test the Tree and Carousel components in isolation. Here's what the carousel looks like right now @ri0nardo. I'll try to get a PR out by end of week, |
@PGrad great start. This is great. I didnt know we agreed on a carousel, but the current design is suppose to redirect you to the photos album. when you hover over an image it tells you the total amount of images available per tree. see attached below. |
@PGrad i thought about the carousel and do like the idea of being able to swipe through the images from the tree info menu. The issue I'm coming across is if we want to show upcoming images or force the user to go through the album. see short clip and image im attaching. |
Is there a way to have the arrows as part of the carousel below the big picture so we can see the entire picture without arrows and carousel blocking the image? Or can we have the thumbnails be a slideable/clickable way to navigate with no arrows? |
You wont know if users will click on the arrows or not. Alot of our questions can be answered if we build and test it out. Depends on the case. Comments on that article label the example as potentially misleading due to adding descriptive labels which i would consider unnecessary for our use case currently. The goal of the carousel is to give the user an option to view more images from the menu. We dont need to force users to use the carousel or add a label. The assumption is that you are going to see another image, not read what the image is. The example i put up is when you hover or the image. the issue I am trying to figure out is how do we make it so mobile users can click without blocking the edges of the image. https://user-images.githubusercontent.com/98299952/230475563-0a354644-c7cb-443e-ad44-4f627d9e40b4.webm |
I'm fine with not having a carousel there and if you click/tap the big image, it goes to the images tab. A lot of carousel haters in the comments of that article so maybe should take it all with a grain of salt. :O If a user takes a picture of say a redwood tree, will/should that picture appear in the images tab gallery for ALL redwood trees or will that only show the photos for that specific location? Probably worth having the image gallery tab divided into "This tree" and "other Locations for this species images" |
Thats the approach I initially thought we were taking so i was caught off guard with a carousel.
Currently I only divided the sections up as
I am not attached to any of the albums, I only used it as a base to go off of. I do like the idea of having other trees in other locations album. |
@ri0nardo I'm thinking that for mobile users, we don't bother with a carousel at all, and just go straight to Photos. I do have the carousel ready to go if we want to use it for desktop. Right now it's just a storybook component and won't go into the app unless we choose to. Getting the photo tab set up will be a separate piece of work. |
I prefer this though I think we can probably go straight to gallery view for mvp because we don't have enough images to fill this out. |
Lets keep the carousel for future use. currently like what rose said we dont have enough images. The mobile aspect is what I was concerned about since I like having as similar experiences both mobile and desktop. Thanks for bringing this up! |
You mentioned google had some sort of AI detector for trees, so I thought we could potentially pull from that if it becomes available for the street view. But its a long shot right now. |
I emailed them a while back and got on some kind of wait list to get them but have heard nothing back, probably worth following up. |
No description provided.
The text was updated successfully, but these errors were encountered: