Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const Wrapper = styled.div`
overflow-y: auto;
border-radius: 8px;
border: solid 1px ${(props) => props.theme.border.border0};
transition: height 0.15s ease;
}
table {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ const MIN_H = 35 * 2;
const MIN_COLUMN_WIDTH = 80;

const TableRow = React.memo(
({ children, item }) => (
<tr key={item.uid} data-testid={`env-var-row-${item.name}`}>
({ children, item, style, ...rest }) => (
<tr key={item.uid} style={style} {...rest} data-testid={`env-var-row-${item?.name}`}>
{children}
</tr>
),
Expand Down Expand Up @@ -56,7 +56,8 @@ const EnvironmentVariablesTable = ({

const hasDraftForThisEnv = draft?.environmentUid === environment.uid;

const [tableHeight, setTableHeight] = useState(MIN_H);
const rowCount = (environment.variables?.length || 0) + 1;
const [tableHeight, setTableHeight] = useState(rowCount * 35);

// Use environment UID as part of tableId so each environment has its own column widths
const tableId = `env-vars-table-${environment.uid}`;
Expand Down Expand Up @@ -483,6 +484,7 @@ const EnvironmentVariablesTable = ({
<TableVirtuoso
className="table-container"
style={{ height: tableHeight }}
overscan={200}
components={{ TableRow }}
data={filteredVariables}
totalListHeightChanged={handleTotalHeightChanged}
Expand All @@ -502,7 +504,6 @@ const EnvironmentVariablesTable = ({
<td></td>
</tr>
)}
fixedItemHeight={35}
computeItemKey={(virtualIndex, item) => `${environment.uid}-${item.index}`}
itemContent={(virtualIndex, { variable, index: actualIndex }) => {
const isLastRow = actualIndex === formik.values.length - 1;
Expand Down Expand Up @@ -535,7 +536,7 @@ const EnvironmentVariablesTable = ({
id={`${actualIndex}.name`}
name={`${actualIndex}.name`}
value={variable.name}
placeholder={!variable.value || (typeof variable.value === 'string' && variable.value.trim() === '') ? 'Name' : ''}
placeholder={!variable.name || (typeof variable.name === 'string' && variable.name.trim() === '') ? 'Name' : ''}
onChange={(e) => handleNameChange(actualIndex, e)}
onFocus={() => handleRowFocus(variable.uid)}
onBlur={() => {
Expand All @@ -560,7 +561,7 @@ const EnvironmentVariablesTable = ({
collection={_collection}
name={`${actualIndex}.value`}
value={variable.value}
placeholder={isLastEmptyRow ? 'Value' : ''}
placeholder={!variable.value || (typeof variable.value === 'string' && variable.value.trim() === '') ? 'Value' : ''}
isSecret={variable.secret}
Comment on lines 563 to 565
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Don't treat 0 and false as empty values here.

!variable.value sends legitimate falsy env values down the placeholder branch. That makes script-set values like 0 and false look empty even though they are valid values.

Suggested fix
-                      placeholder={!variable.value || (typeof variable.value === 'string' && variable.value.trim() === '') ? 'Value' : ''}
+                      placeholder={
+                        variable.value == null || (typeof variable.value === 'string' && variable.value.trim() === '')
+                          ? 'Value'
+                          : ''
+                      }

Based on learnings "falsy values are legitimate environment variable values and should not be filtered out."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js` around
lines 563 - 565, The placeholder logic in EnvironmentVariablesTable treats falsy
values like 0 and false as empty because it uses !variable.value; update the
placeholder expression to only consider null/undefined or blank strings as
empty. Specifically, replace the !variable.value check in the placeholder prop
for the variable.value field with an explicit check like variable.value ===
undefined || variable.value === null || (typeof variable.value === 'string' &&
variable.value.trim() === '') so numeric 0 and boolean false are preserved as
real values.

readOnly={typeof variable.value !== 'string'}
onChange={(newValue) => {
Expand All @@ -570,6 +571,19 @@ const EnvironmentVariablesTable = ({
formik.setFieldValue(`${actualIndex}.ephemeral`, undefined, false);
formik.setFieldValue(`${actualIndex}.persistedValue`, undefined, false);
}
// Append a new empty row when editing value on the last row
if (isLastRow) {
setTimeout(() => {
formik.setFieldValue(formik.values.length, {
uid: uuid(),
name: '',
value: '',
type: 'text',
secret: false,
enabled: true
}, false);
}, 0);
}
}}
onSave={handleSave}
/>
Expand Down Expand Up @@ -610,6 +624,8 @@ const EnvironmentVariablesTable = ({
/>
)}

{/* We should re-think of these buttons placement in component as we use TableVirtuoso which because of
these buttons renders at some transition: height 0.1s ease` */}
<div className="button-container">
<div className="flex items-center">
<button type="button" className="submit" onClick={handleSave} data-testid="save-env">
Expand Down
Loading