-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
added the ability to add a photo in the item creation modal #173
added the ability to add a photo in the item creation modal #173
Conversation
WalkthroughWalkthroughThe recent modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateModal
participant FileReader
participant Server
User->>CreateModal: Clicks upload label
CreateModal-->>User: Show file input
User->>CreateModal: Selects image file
CreateModal->>FileReader: Read file
FileReader-->>CreateModal: Return image data URL
CreateModal-->>User: Display image preview
User->>CreateModal: Submits form
CreateModal->>Server: Upload image
Server-->>CreateModal: Confirm upload success
CreateModal-->>User: Show success message
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
frontend/components/Item/CreateModal.vue (2)
11-16
: Ensure accessibility for the hidden input.The hidden file input is triggered by a label, which is a good practice for cleaner UI. Ensure that the label is accessible to screen readers for better accessibility.
- <label for="photo" class="btn">Photo 📷</label> + <label for="photo" class="btn" aria-label="Upload Photo">Photo 📷</label>
41-51
: Ensure consistent styling and add alt text for images.The preview area provides immediate feedback, which is excellent for user experience. Ensure the styling is consistent with the rest of the application and consider adding alt text for accessibility.
- <img :src="form.preview" class="h-[100px] w-full object-cover rounded-t shadow-sm border-gray-300" /> + <img :src="form.preview" alt="Image Preview" class="h-[100px] w-full object-cover rounded-t shadow-sm border-gray-300" />
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- frontend/components/Item/CreateModal.vue (5 hunks)
Additional comments not posted (3)
frontend/components/Item/CreateModal.vue (3)
113-114
: Form data structure changes look good.The addition of
preview
andphoto
properties is appropriate for managing image data. Ensure these properties are reset correctly after form submission.
132-144
: Function implementation looks good.The
uploadImage
function correctly checks for file existence before attempting an upload. Ensure this function is integrated appropriately within the component logic.
191-201
: Verify error handling and consider logging.The
create
function correctly handles photo uploads and provides user feedback. Ensure error handling is robust and consider adding logging for debugging purposes.
Comments failed to post (1)
frontend/components/Item/CreateModal.vue
119-129: Consider adding error handling for FileReader.
The
previewImage
function effectively uses the FileReader API to preview images. Implement error handling to manage potential file reading errors gracefully.reader.onerror = () => { console.error("Error reading file"); toast.error("Failed to load image preview"); };
Sorry for the stupid question, but is this checked in to the main branch so all I have to do is install? Or is there something more I have to do (or other branch I should use)? To me, this is core functionality, so I really want to make it happen. |
@don-ferris You have a couple of options. The easiest is to checkout this PR's branch and then use @cjmielke @tankerkiller125 I tested the functionality. It works as expected, and it already made using homebox much easier and more enjoyable. The placement of the button and its styling isn't great, but I'm not a designer. |
@adiov I hear ya, I'm no designer either, and I try to avoid frontend as much as I can. I tried working on that button's margins for a while, but then said "eff it, it works". CSS always fights me. @don-ferris I just merged it to my main branch, making it a bit more straightforward. Until the devs merge it, you can get my copy up by doing the following. I'll be hosting this branch on my own eventually, once I have time to migrate my existing server.
|
Thank you for that! When I read your previous reply, I was like "no way I'm going to be able to pull that off!" Way above my pay grade! So I tried your instrux and got this:
No idea what to do about that. Can I get a little help? Update: I tried using the standard docker-compose and replacing (image: ghcr.io/)"sysadminsmedia" with "cjmielke" and I got this: Update 2: So, the first time, I copied all 4 lines, pasted them, and hit enter. So I nuked the homebox directory and started over. Beginning with the git clone command, I executed each one individually. No errors on the docker compose build command but...
Hope that helps you to help me. (Thanks again, by the way. I really appreciate you!) P.S. Once I get this working, I'll do a PR on your repo and fix the CSS to make it look pretty. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good, I'm not exactly a UI dev myself, worse case this can be cleaned up a later point.
I have just two little nitpicks I'd like to see sorted before I merge this in, just some cleanup and basic translation stuff.
Thanks for the PR!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- frontend/components/Item/CreateModal.vue (5 hunks)
- frontend/locales/en.json (1 hunks)
Additional comments not posted (9)
frontend/locales/en.json (1)
21-22
: Addition of new key-value pair for photo button.The new key "photo_button" with the value "Photo 📷" has been added correctly to the "create_modal" section.
The code changes are approved.
frontend/components/Item/CreateModal.vue (8)
9-10
: Addition of new file input element for photo upload.The new file input element and label for the photo button have been added correctly, enhancing the user interface.
The code changes are approved.
13-16
: Addition of new label and input for photo upload.The new label and input for photo upload are correctly implemented, providing a user-friendly way to upload images.
The code changes are approved.
38-44
: Addition of new div for photo preview area.The new div for the photo preview area is correctly implemented, allowing users to see a preview of the uploaded image.
The code changes are approved.
106-107
: Addition of new reactive properties for image upload.The new reactive properties
preview
andphoto
have been added correctly to the form object, enabling image upload functionality.The code changes are approved.
112-122
: Addition of new function to preview image.The new function
previewImage
is correctly implemented, allowing users to preview the uploaded image.The code changes are approved.
125-137
: Addition of new function to upload image.The new function
uploadImage
is correctly implemented, handling the image upload process.The code changes are approved.
184-194
: Update to the create function to handle image uploads.The
create
function has been updated correctly to handle image uploads, ensuring that the image is uploaded after the item is created.The code changes are approved.
201-202
: Resetting form properties after item creation.The form properties
preview
andphoto
are reset correctly after the item is created, ensuring a clean state for the next item creation.The code changes are approved.
@cjmielke Could you please edit your PR to use one of the closing keywords? This will assign the bounty to you. |
@don-ferris now that they merged it into main, you might be able to just update your existing install Aside from this, not sure about the error you are experiencing. Maybe you need superuser permissions on your specific system, but its just a guess. |
You would have to run the :nightly build instead of :latest for docker. :latest only contains the tagged release, although we're planning on doing that fairly soon. |
Either I'm doing something wrong or there's a problem with the latest nightly build.
And here's the command/error output:
I did an update/upgrade to make sure my system is totally up to date. Help? |
@don-ferris You might be using the older "docker-compose" command, but as I learned pretty recently, the now builtin "docker compose" is the new version https://docs.docker.com/compose/migrate/ I recently had a different issue, but that was the fix. "docker SPACE compose" instead of "docker DASH compose". The builtin command is much more up to date. If that fixes your problem, you might consider uninstalling the older docker-compose package. Thats what I did to prevent myself from ever making that mistake again. |
That error is 100% related to the older docker-compose command which is no longer supported and is EOL. We're now on docker compose version 2 something rather using the |
… v0.14.0@9f47d0f by renovate (#25934) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/sysadminsmedia/homebox](https://redirect.github.com/sysadminsmedia/homebox) | minor | `0.13.0` -> `0.14.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>sysadminsmedia/homebox (ghcr.io/sysadminsmedia/homebox)</summary> ### [`v0.14.0`](https://redirect.github.com/sysadminsmedia/homebox/releases/tag/v0.14.0) [Compare Source](https://redirect.github.com/sysadminsmedia/homebox/compare/v0.13.0...v0.14.0) > \[!NOTE]\ > We are aware of some users having issues with the v0.12.0 -> v0.13.0 update on docker specifically, we have so far been unable to replicate the issue and as such this version does not contain a patch for it. We want to reassure you though that we are committed to getting to the bottom of that issue and fixing it, whether that be a patch release, or something that goes in a fuller release. #### Homebox Goes International (Languages) This update contains many fixes, and small resolutions, but it also contains a HUGE update in terms of making Homebox more accessible to international users. We've finally added i18n support to Homebox, allowing users to use Homebox in their own language. It's still early days, we've translated the majority of the main pages, and we continue working on it. To use translations, simply open Homebox, it's really that simple, it will use whatever the default language setting your browser is set to use. And fallback to English if your preferred language isn't yet available. If your language isn't yet available, please consider [contributing](https://translate.sysadminsmedia.com/projects/homebox/frontend/). For those that have already contributed, both those named, and those that don't have your GitHub linked in Weblate (and thus don't appear in commit messages to mention) thank you so much, we couldn't possible translate Homebox into so many languages ourselves. [![Translation status](http://translate.sysadminsmedia.com/widget/homebox/multi-auto.svg)](http://translate.sysadminsmedia.com/engage/homebox/) Additionally, we purchased the https://homebox.software domain to give Homebox a home of it's own on the internet. Which should not only make it more findable for SEO reasons, but also make it easier to remember the link to the documentation/home page. Additionally, we setup some sub-domains to redirect to various sites such as https://git.homebox.software for Github, https://discord.homebox.software to go to our Discord server, etc. #### What's Changed - Fix small typo in label discussion thread URL by [@​victorhooi](https://redirect.github.com/victorhooi) in [https://github.com/sysadminsmedia/homebox/pull/149](https://redirect.github.com/sysadminsmedia/homebox/pull/149) - Document search tips, cleanup documentation by [@​blessedbiped](https://redirect.github.com/blessedbiped) in [https://github.com/sysadminsmedia/homebox/pull/152](https://redirect.github.com/sysadminsmedia/homebox/pull/152) - Fixes to the Tools page by [@​rpavlik](https://redirect.github.com/rpavlik) in [https://github.com/sysadminsmedia/homebox/pull/154](https://redirect.github.com/sysadminsmedia/homebox/pull/154) - Adding i18n initial support by [@​tankerkiller125](https://redirect.github.com/tankerkiller125) in [https://github.com/sysadminsmedia/homebox/pull/155](https://redirect.github.com/sysadminsmedia/homebox/pull/155) - changed companyname and url to sysadminsmedia.com by [@​101br03k](https://redirect.github.com/101br03k) in [https://github.com/sysadminsmedia/homebox/pull/158](https://redirect.github.com/sysadminsmedia/homebox/pull/158) - \[LANGUAGE UPDATE] Frontend translations for Italian and German by [@​lukasitaly](https://redirect.github.com/lukasitaly) in [https://github.com/sysadminsmedia/homebox/pull/170](https://redirect.github.com/sysadminsmedia/homebox/pull/170) - Adding email validator by [@​RomuloGatto](https://redirect.github.com/RomuloGatto) in [https://github.com/sysadminsmedia/homebox/pull/178](https://redirect.github.com/sysadminsmedia/homebox/pull/178) - fix: CSV export not including item notes by [@​LarssonOliver](https://redirect.github.com/LarssonOliver) in [https://github.com/sysadminsmedia/homebox/pull/180](https://redirect.github.com/sysadminsmedia/homebox/pull/180) - added the ability to add a photo in the item creation modal by [@​cjmielke](https://redirect.github.com/cjmielke) in [https://github.com/sysadminsmedia/homebox/pull/173](https://redirect.github.com/sysadminsmedia/homebox/pull/173) #### New Contributors - [@​victorhooi](https://redirect.github.com/victorhooi) made their first contribution in [https://github.com/sysadminsmedia/homebox/pull/149](https://redirect.github.com/sysadminsmedia/homebox/pull/149) - [@​blessedbiped](https://redirect.github.com/blessedbiped) made their first contribution in [https://github.com/sysadminsmedia/homebox/pull/152](https://redirect.github.com/sysadminsmedia/homebox/pull/152) - [@​rpavlik](https://redirect.github.com/rpavlik) made their first contribution in [https://github.com/sysadminsmedia/homebox/pull/154](https://redirect.github.com/sysadminsmedia/homebox/pull/154) - [@​lukasitaly](https://redirect.github.com/lukasitaly) made their first contribution in [https://github.com/sysadminsmedia/homebox/pull/170](https://redirect.github.com/sysadminsmedia/homebox/pull/170) - [@​RomuloGatto](https://redirect.github.com/RomuloGatto) made their first contribution in [https://github.com/sysadminsmedia/homebox/pull/178](https://redirect.github.com/sysadminsmedia/homebox/pull/178) - [@​LarssonOliver](https://redirect.github.com/LarssonOliver) made their first contribution in [https://github.com/sysadminsmedia/homebox/pull/180](https://redirect.github.com/sysadminsmedia/homebox/pull/180) - [@​cjmielke](https://redirect.github.com/cjmielke) made their first contribution in [https://github.com/sysadminsmedia/homebox/pull/173](https://redirect.github.com/sysadminsmedia/homebox/pull/173) #### Translation Contributors Please note, this list is based on Github Commits from Weblate, it may not be 100% accurate, or contain all contributors. - [@​SKNTim](https://redirect.github.com/SKNTim) (Chinese Traditional) - [@​olsson82](https://redirect.github.com/olsson82) (Chinese Traditional) - [@​101br03k](https://redirect.github.com/101br03k) (Chinese Traditional, Swedish, French) - [@​Jackxwb](https://redirect.github.com/Jackxwb) (Chinese Simplified) - [@​SodaSyrup](https://redirect.github.com/SodaSyrup) (Turkish) - [@​N0namenull](https://redirect.github.com/N0namenull) (Russian) - [@​scyllaL](https://redirect.github.com/scyllaL) (Russian) - [@​Slydite4](https://redirect.github.com/Slydite4) (Spanish) - [@​HydrelioxGitHub](https://redirect.github.com/HydrelioxGitHub) (French) - [@​chevdor](https://redirect.github.com/chevdor) (French) - [@​lukasitaly](https://redirect.github.com/lukasitaly) (Italian, German, French) - [@​olsson82](https://redirect.github.com/olsson82) (Swedish, German) - [@​terenc3](https://redirect.github.com/terenc3) (German) **Full Changelog**: sysadminsmedia/homebox@v0.13.0...v0.14.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://redirect.github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC42Ny40IiwidXBkYXRlZEluVmVyIjoiMzguNjcuNCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciIsImxhYmVscyI6WyJhdXRvbWVyZ2UiLCJ1cGRhdGUvZG9ja2VyL2dlbmVyYWwvbm9uLW1ham9yIl19-->
What type of PR is this?
What this PR does / why we need it:
Adds a button to the item creation model to add a photograph, so that users don't have to dig for the option in the advanced edit menu afterwards. I've been wanting this feature for a while too, and apparently others do as well.
Which issue(s) this PR fixes:
Fixes: #58
Special notes for your reviewer:
This is my first time developing in Vue.js, so apologies
A better solution might be to also modify the backend to accept the image on item creation, but for simplicity I just post'd a second API call from the frontend using the same function as the edit menu.
Testing
Works on my end, and Ive done my best to make sure it works in tiny phone screens. The UI could be improved.
Summary by CodeRabbit
New Features
Bug Fixes
closes #58