Feature/transfer card optional with info #25
Conversation
cachebag
added
frontend
cachebag
left a comment
cachebag
left a comment
There was a problem hiding this comment.
I actually really like this feature, thanks!
I should note, as I mentioned here, while I am happy to include new features, I think it's wise to start thinking about whether we should force it as the default for the app.
As such, with features like these, and any other future ones, we should consider making them optional entirely. Similar to how you did here, but not simply disabling/enabling for edits, but rather completely hiding the feature from users if they prefer not to use them.
| setEditingId(item.id); | ||
| setDescription(item.description); | ||
| setAmount(item.amount.toString()); | ||
| setCategoryId(item.category_id.toString()); |
There was a problem hiding this comment.
Not sure why we need the category form for transfers? 🤔 It doesn't make sense to set a transfer to go towards savings or retirement, and then be able to pick from our budget options? I don't think it should be here
| <div className="bg-sand-100 dark:bg-charcoal-900 p-4 sm:p-6 border border-sand-200 dark:border-charcoal-800"> | ||
| <h2 className="text-base sm:text-lg font-medium mb-4 text-charcoal-800 dark:text-sand-100"> | ||
| Transferred Items | ||
| </h2> | ||
| <div className="space-y-4"> | ||
| <div className="flex items-center justify-between"> | ||
| <div className="flex-1"> | ||
| <div className="flex items-center gap-2 mb-1"> | ||
| <label className="text-sm font-medium text-charcoal-700 dark:text-sand-300"> | ||
| Enable Transferred Items | ||
| </label> | ||
| <button | ||
| onClick={() => setShowTransfersModal(true)} | ||
| className="p-0.5 hover:bg-sand-200 dark:hover:bg-charcoal-700 rounded transition-colors touch-manipulation" | ||
| title="How to use transfers" | ||
| > | ||
| <Info size={14} className="text-charcoal-400 hover:text-charcoal-600 dark:hover:text-charcoal-300" /> | ||
| </button> | ||
| </div> | ||
| <p className="text-xs text-charcoal-500 dark:text-charcoal-400"> | ||
| Allow adding, editing, and deleting transferred items | ||
| </p> | ||
| </div> | ||
| <button | ||
| onClick={() => setTransfersEnabled(!transfersEnabled)} | ||
| className={`relative inline-flex h-6 w-11 items-center rounded-full transition-colors ${ | ||
| transfersEnabled | ||
| ? "bg-sage-600 dark:bg-sage-500" | ||
| : "bg-charcoal-300 dark:bg-charcoal-600" | ||
| }`} | ||
| > | ||
| <span | ||
| className={`inline-block h-4 w-4 transform rounded-full bg-white transition-transform ${ | ||
| transfersEnabled ? "translate-x-6" : "translate-x-1" | ||
| }`} | ||
| /> | ||
| </button> | ||
| </div> | ||
| </div> </div> | ||
|
|
There was a problem hiding this comment.
So this disables or enables editing the transfers but I think what makes more sense for the sake of this project is to actually just hide it entirely. If people don't want to manage their transfers, they shouldn't need to see that section at all in the dashboard.
There was a problem hiding this comment.
I have changed it so if you disable the feature it removes the card if there are no transfers, if there are transfers it remains until the transfers are deleted.
Can change if preferred?
cachebag
force-pushed
the
feature/transfer-card-optional-with-info
branch
from
21a6fbc to
b587655
Compare
|
I saw the conversation and agree, was looking at it as you also mentioned seperating them in the previous PR and thought it would be better. I will look into disabling completely in the options. |
2 participants
Enhancements to PR #20