fix 171 Wrong tooltip for toggle settings #178
Conversation
There was a problem hiding this comment.
Pull request overview
Updates settings UI tooltip behavior and improves local/runtime configuration handling.
Changes:
- Replace hardcoded boolean-toggle tooltip with a dynamic tooltip derived from the setting metadata.
- Load environment variables for Sequelize DB config via
dotenvfrom the repo root. - Update
.gitignoreto ignore a top-levellogs/directory.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/components/dashboard/settings/SettingItem.vue | Uses a computed tooltip for boolean toggles and adds a helper method for the title text. |
| backend/db/config/config.js | Loads .env before exporting Sequelize configuration so process.env.* values resolve. |
| .gitignore | Adds logs/ ignore and adjusts backend section comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -33,7 +33,7 @@ | |||
| :checked="setting.value" | |||
There was a problem hiding this comment.
The checkbox uses v-model="setting.value" and also binds :checked="setting.value". In Vue, v-model already controls the checked state for checkboxes, so the extra :checked binding is redundant and can lead to confusing/duplicated sources of truth. Consider removing the :checked binding and rely on v-model alone.
| :checked="setting.value" |
| getBooleanToggleTooltip(setting){ | ||
| const label = (setting.description && setting.description.trim()) || setting.key; | ||
| return `${label}`; |
There was a problem hiding this comment.
getBooleanToggleTooltip currently just returns the computed label via a template literal. Consider returning label directly (no interpolation needed) and matching the spacing/style used by the other methods (space before {) to keep formatting consistent in this file.
| getBooleanToggleTooltip(setting){ | |
| const label = (setting.description && setting.description.trim()) || setting.key; | |
| return `${label}`; | |
| getBooleanToggleTooltip(setting) { | |
| const label = (setting.description && setting.description.trim()) || setting.key; | |
| return label; |
| * @author Nils Dycke | ||
| */ | ||
| const path = require("path"); | ||
| require("dotenv").config({path: path.resolve(__dirname, "../../../.env")}); |
There was a problem hiding this comment.
This hard-codes loading ../../../.env. The repo supports environment-specific files via ENV (see Makefile includes .env.${ENV}), but this config will always read only .env when env vars aren’t already set (e.g., running backend/db scripts directly). Consider loading .env.${process.env.ENV} when ENV is set (falling back to .env), or relying on the parent process to provide env vars to avoid surprising configuration mismatches.
| require("dotenv").config({path: path.resolve(__dirname, "../../../.env")}); | |
| const envFileName = process.env.ENV ? `.env.${process.env.ENV}` : ".env"; | |
| require("dotenv").config({path: path.resolve(__dirname, "../../../", envFileName)}); |
|
I have a question. For now, the tooltip is the description of the toggle settings (the line below the title of the setting). I am currently thinking of a way to make the tooltip custom for each and every setting, instead of just copying the description. Could anyone suggest me how can we do it? |
dennis-zyska
left a comment
There was a problem hiding this comment.
Please wait with this implementation, Mohammad is currently refactoring the settings page
| * @author Nils Dycke | ||
| */ | ||
| const path = require("path"); | ||
| require("dotenv").config({path: path.resolve(__dirname, "../../../.env")}); |
There was a problem hiding this comment.
Not sure why you do that here, is it needed?
|
|
||
| # backend | ||
| logs/ | ||
| # backen |
| } | ||
| }, | ||
| methods: { | ||
| getBooleanToggleTooltip(setting){ |
There was a problem hiding this comment.
the name of the function seems misleading
Wrong tooltip for toggle settings
What is the error?
The error was that the same tooltip option was being displayed on all the toggle settings in the settings tab. Which does not make sense.
Where the error was.
This error is seen in
SettingItem.vue, specifically here.The fix
Here, I changed the tile field to evaluate from the description of the settings. Please see the changes to get a clear understanding of what I did.