mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-09 15:35:39 +02:00
fix(projects): make edit modal scrollable so Save is reachable on short screens (#1642)
Reporter on a 1508x831 Pi display couldn't mark a project as Completed because the edit modal's height exceeded the viewport: the outer wrapper centers vertically and the inner card had no max-h and no overflow, so the top half scrolled above and the bottom half (Status dropdown + Save/Cancel) scrolled below. Workaround was a full page reload. Standard flex-modal-scroll fix: max-h-[calc(100vh-2rem)] + flex flex-col on the card; a flex-1 overflow-y-auto min-h-0 wrapper around the form fields; Cancel/Save moved into a flex-shrink-0 sibling with a border-t separator so they're always visible regardless of scroll position. Buttons stay inside <form> so type="submit" still works.
This commit is contained in:
@@ -4,6 +4,9 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
|
||||
## [0.2.5b1] - Unreleased
|
||||
|
||||
### Fixed
|
||||
- **Project edit modal couldn't be scrolled, so Save / Cancel were unreachable on short screens (#1642, reported by @klevin92)** — Reporter on a Pi-class display (1508 × 831) couldn't mark a project as Completed because the edit modal's height exceeded the viewport and there was no way to scroll: outer wrapper was `fixed inset-0 flex items-center justify-center p-4` (vertical-center) and the inner card had no `max-h` and no `overflow`. The top of the form went above the viewport and the bottom — including the Status dropdown the reporter was trying to use plus both action buttons — went below it. Workaround was a full page reload to drop the modal. Standard flex-modal-scroll fix: `max-h-[calc(100vh-2rem)]` + `flex flex-col` on the card (the `2rem` accounts for the outer `p-4`), a `flex-1 overflow-y-auto min-h-0` wrapper around the form fields, and the Cancel / Save buttons moved into a `flex-shrink-0` sibling with a `border-t` separator so they become a sticky footer that's always visible regardless of scroll position. The buttons stay inside the `<form>` so `type="submit"` still works. 2 new vitest cases in `ProjectsPage.test.tsx` pin the structural fix: the Save button is NOT a descendant of the `overflow-y-auto` region (otherwise it would scroll off again) and the modal card carries the `max-h-[calc(100vh-2rem)]` cap. Other modals in the codebase with the same `fixed inset-0 flex items-center justify-center` + `max-w-md` shape almost certainly have the same latent bug — not refactored here, will tackle when reported.
|
||||
|
||||
### Changed
|
||||
- **File Manager sidebar: "All Files" now scopes to your own uploaded files; new "External" entry holds the combined linked-folder view (#1621, reported by @kcw96)** — Reporter linked a NAS share that auto-imported hundreds of 3MFs, and from then on their handful of Bambuddy-uploaded files was lost in the "All Files" listing — no filter, no toggle, only per-folder clicks to escape the noise. Restored the pre-external semantics so long-time users get their muscle memory back: "All Files" lists managed-storage files only (`is_external=False`), exactly what it meant before external folders existed. The combined "everything across every external mount" view moves to a new sibling sidebar entry, **External**, which only renders when at least one external folder is linked (zero-cost on installs that don't use the feature). Per-folder clicking is unchanged: clicking any folder in the tree — internal or external — still shows that folder's contents directly. **Backend**: `/api/v1/library/files` gains two mutually-exclusive query flags, `internal_only` and `external_only`, filtering directly on `LibraryFile.is_external`. Both-flags-set is a 400 (catches frontend regressions immediately instead of silently picking one). Folder- or project-scoped requests bypass both flags because they already imply a single scope. **Frontend**: new `topLevelView: 'internal' | 'external'` state on `FileManagerPage`, default `internal`; the query passes the corresponding scope only when `selectedFolderId === null`. Sidebar shows the "External" row gated on `folders.some(f => f.is_external)`; mobile selector dropdown carries a `__top:internal` / `__top:external` sentinel so the same state can round-trip through `<option value>`. Empty-state copy distinguishes "no internal files yet" from "no external files" so a user staring at an empty External view doesn't think their NAS is broken. **i18n**: 3 new keys (`allExternal`, `externalIsEmpty`, `externalEmptyDescription`) translated in all 10 non-English locales. **Tests**: 3 new backend integration tests in `test_library_api.py` (internal-only with mixed root + folder + external file mix, external-only across two NAS mounts, mutually-exclusive 400) and 3 new frontend tests in `FileManagerPage.test.tsx` (External entry conditional on `is_external`, default internal-only query, External-click switches scope). Existing 48 `FileManagerPage` tests + 11 `FileManagerExternalFolder` tests stay green. **Behaviour change for the small set of users who relied on the combined view as default**: clicking "External" once gets the previous union behaviour (across-all-externals); clicking a specific external folder still shows just that mount, same as before.
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { screen, waitFor } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { render } from '../utils';
|
||||
import { ProjectsPage } from '../../pages/ProjectsPage';
|
||||
import { ProjectsPage, ProjectModal } from '../../pages/ProjectsPage';
|
||||
import { http, HttpResponse } from 'msw';
|
||||
import { server } from '../mocks/server';
|
||||
|
||||
@@ -305,4 +305,78 @@ describe('ProjectsPage', () => {
|
||||
expect(document.querySelectorAll('[aria-hidden="true"] img').length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('modal scrolls on short viewports (#1642)', () => {
|
||||
/**
|
||||
* Reporter on a Pi screen couldn't reach the Save button when editing a
|
||||
* project because the modal had no max-h / overflow. The structural fix
|
||||
* puts a max-h on the card, the form fields in a `flex-1 overflow-y-auto`
|
||||
* wrapper, and the Save/Cancel buttons in a `flex-shrink-0` sibling so
|
||||
* they're always visible regardless of scroll position.
|
||||
*
|
||||
* jsdom doesn't compute layout heights so we can't simulate the actual
|
||||
* overflow. We pin the structure instead: the scrollable wrapper exists,
|
||||
* the Save button is NOT a descendant of it, and the card has a max-h.
|
||||
* A future refactor that removes any of these would re-introduce the bug.
|
||||
*/
|
||||
const editableProject = {
|
||||
id: 7,
|
||||
name: 'Spool holder',
|
||||
description: null,
|
||||
color: '#00ae42',
|
||||
url: null,
|
||||
cover_image_filename: null,
|
||||
archive_count: 0,
|
||||
total_print_time_seconds: 0,
|
||||
total_filament_grams: 0,
|
||||
target_plates_count: null,
|
||||
target_parts_count: null,
|
||||
tags: null,
|
||||
due_date: null,
|
||||
priority: null,
|
||||
budget: null,
|
||||
status: 'active' as const,
|
||||
created_at: '2024-01-01T00:00:00Z',
|
||||
updated_at: '2024-01-01T00:00:00Z',
|
||||
};
|
||||
|
||||
it('renders the action footer outside the scrollable fields wrapper', () => {
|
||||
render(
|
||||
<ProjectModal
|
||||
project={editableProject}
|
||||
onClose={() => {}}
|
||||
onSave={() => {}}
|
||||
isLoading={false}
|
||||
currencySymbol="€"
|
||||
t={((k: string) => k) as never}
|
||||
/>,
|
||||
);
|
||||
|
||||
const saveButton = screen.getByRole('button', { name: 'common.save' });
|
||||
const scrollable = document.querySelector('.overflow-y-auto');
|
||||
expect(scrollable).not.toBeNull();
|
||||
// The save button must live OUTSIDE the scrollable region — otherwise
|
||||
// a long form pushes it below the fold on short viewports (#1642).
|
||||
expect(scrollable!.contains(saveButton)).toBe(false);
|
||||
});
|
||||
|
||||
it('caps the modal card height so it cannot exceed the viewport', () => {
|
||||
render(
|
||||
<ProjectModal
|
||||
project={editableProject}
|
||||
onClose={() => {}}
|
||||
onSave={() => {}}
|
||||
isLoading={false}
|
||||
currencySymbol="€"
|
||||
t={((k: string) => k) as never}
|
||||
/>,
|
||||
);
|
||||
|
||||
// Card has max-h set so it never extends past the viewport — without
|
||||
// this, vertical-center alignment pushes the bottom of the modal
|
||||
// (including the action footer) off-screen.
|
||||
const card = document.querySelector('.max-h-\\[calc\\(100vh-2rem\\)\\]');
|
||||
expect(card).not.toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -133,15 +133,19 @@ export function ProjectModal({ project, onClose, onSave, isLoading, currencySymb
|
||||
};
|
||||
|
||||
return (
|
||||
// max-h + flex column on the card + overflow on the fields wrapper so the
|
||||
// modal stays inside the viewport on short screens (#1642). Outer p-4 is
|
||||
// 1rem each side, hence the 2rem subtraction below.
|
||||
<div className="fixed inset-0 bg-black/70 flex items-center justify-center z-50 p-4">
|
||||
<div className="bg-bambu-dark-secondary rounded-lg w-full max-w-md border border-bambu-dark-tertiary">
|
||||
<div className="p-4 border-b border-bambu-dark-tertiary">
|
||||
<div className="bg-bambu-dark-secondary rounded-lg w-full max-w-md border border-bambu-dark-tertiary flex flex-col max-h-[calc(100vh-2rem)]">
|
||||
<div className="p-4 border-b border-bambu-dark-tertiary flex-shrink-0">
|
||||
<h2 className="text-lg font-semibold text-white">
|
||||
{project ? t('projects.editProject') : t('projects.newProject')}
|
||||
</h2>
|
||||
</div>
|
||||
|
||||
<form onSubmit={handleSubmit} className="p-4 space-y-4">
|
||||
<form onSubmit={handleSubmit} className="flex flex-col flex-1 min-h-0">
|
||||
<div className="p-4 space-y-4 overflow-y-auto flex-1">
|
||||
<div>
|
||||
<label className="block text-sm font-medium text-white mb-1">
|
||||
{t('common.name')}
|
||||
@@ -374,8 +378,12 @@ export function ProjectModal({ project, onClose, onSave, isLoading, currencySymb
|
||||
</select>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className="flex justify-end gap-2 pt-2">
|
||||
{/* Sticky action footer — stays visible regardless of scroll
|
||||
position so Save/Cancel are always reachable on short screens
|
||||
(#1642). Buttons stay inside <form> for type="submit". */}
|
||||
<div className="flex justify-end gap-2 p-4 border-t border-bambu-dark-tertiary flex-shrink-0">
|
||||
<Button type="button" variant="secondary" onClick={onClose}>
|
||||
{t('common.cancel')}
|
||||
</Button>
|
||||
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -26,7 +26,7 @@
|
||||
|
||||
<!-- Splash screens for iOS -->
|
||||
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
|
||||
<script type="module" crossorigin src="/assets/index-DrAXd6Gv.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-CyGvoJrx.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-Df3XYvpK.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user