Make modal footer fixed (#6696) #6699

XanderLuciano · 2024-03-13T09:44:08Z

Quick fix for #6696

Does not work great on low vertical resolution screens, need to probably calculate max view size in pixels for device and use that to fix div height. I was having trouble making the outer modal container fill to viewport height AND scroll as expected.

Another option is to add a ton of padding / margin to the bottom of the form and make the bottom footer fixed.

There's a few ways to approach this in a more "proper" way. But wanted to get an early and semi-functional scroll in case someone else wants to suggest some improvements that build on this.

Otherwise, this is a short term fix to make the bottom bar a little nicer to use again.

Closes #6696

netlify · 2024-03-13T09:44:23Z

✅ Deploy Preview for inventree-web-pui-preview ready!

Name	Link
🔨 Latest commit	`007b888`
🔍 Latest deploy log	https://app.netlify.com/sites/inventree-web-pui-preview/deploys/65faa018400d900008a3cab6
😎 Deploy Preview	https://deploy-preview-6699--inventree-web-pui-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (no change from production) Accessibility: 86 (no change from production) Best Practices: 100 (no change from production) SEO: 70 (no change from production) PWA: - View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

XanderLuciano · 2024-03-13T09:49:33Z

SchrodingersGat · 2024-03-13T10:11:26Z

@XanderLuciano looks great! I'm happy to merge this as-is, unless you are going to work on the "improvements" right away?

wolflu05 · 2024-03-13T11:00:45Z

Thanks for that fix, that is looking really promising, I remember I was trying this too, but did not succeed.

But when using the APIForm without a height restricted modal, e.g. in the Admin center in the user modal we have a scroll bar too at the wrong place. Instead if it overflows we should be able scroll in the drawer, otherwise we would end up with two scroll bars if e.g. the groups list gets to long.

Before	Now

matmair

LGTM; thank you @XanderLuciano

SchrodingersGat · 2024-03-13T21:23:23Z

@XanderLuciano can you please address the comments by @wolflu05 here? :)

SchrodingersGat · 2024-03-14T03:57:37Z

One thing I have noticed here (not introduced by your PR, but just with forms in general).

When loading the form initially, the form displays with a big empty gap between the header and the footer. It would be nice to display a <Skeleton /> or <Loader /> in the middle of the form whilst the initial content is fetching? At the very least, get rid of the ugly "gap"

XanderLuciano · 2024-03-14T07:16:02Z

Originally was going to say I was happy enough with this PR as-is, but the issues pointed out by @wolflu05 are regressions I would like to avoid.

I think I can come up with a better solution that improves the behavior in more places, and solves the empty modal appearance on initial load.

Good comments all around detailing the small issues. I’d like to refine this a little further before we merge this. :)

commit 06c7ebf Author: Oliver <oliver.henry.walters@gmail.com> Date: Sat Mar 16 09:11:57 2024 +1100 Update docker_install.md (inventree#6723) * Update docker_install.md Add note about external access * Update docker_install.md commit a00d5ab Author: Oliver <oliver.henry.walters@gmail.com> Date: Fri Mar 15 17:53:58 2024 +1100 Disable BOM requirement (inventree#6719) * Add new setting STOCK_ENFORCE_BOM_INSTALLATION - Defaults to True (legacy) * Add logic to bypass BOM check * Update CUI to reflect new logic * Render InstalledItemsTable in PUI commit 160d014 Author: Oliver <oliver.henry.walters@gmail.com> Date: Fri Mar 15 17:12:53 2024 +1100 [PUI] Details Pages (inventree#6718) * Add "details" view to SupplierPart page * Fix PartActions * Add placeholder for actions * Add "title" option to DetailsTable * Add edit form to supplier part page * Fix link to manufacturer part * Add "details" view to ManufacturerPartDetail page * Add edit for ManufacturerPart * Create new manufacturer part from company table * Tweak ActionIcon commit 57a1a81 Author: Oliver <oliver.henry.walters@gmail.com> Date: Fri Mar 15 12:24:17 2024 +1100 Reporting: Build line label fix (inventree#6717) * Fix "BuildLine" label in PUI - Point to "buildline" not "build" * Prevent escape closing template ediror * Update report docs * Fix for format_number - Prevent number from being represented as scientific notation commit 0196dd2 Author: Lavissa <lavissawow@gmail.com> Date: Fri Mar 15 02:06:18 2024 +0100 [PUI/Feature] Integrate Part "Default Location" into UX (inventree#5972) * Add default parts to location page * Fix name strings * Add Stock Transfer modal * Add ApiForm Table field * temp * Add stock transfer form to part, stock item and location * All stock operations for Item, Part, and Location added (except order new) * Add default_location category traversal, and initial PO Line Item Receive form * . * Remove debug values * Added PO line receive form * Add functionality to PO receive extra fields * . * Forgot to bump API version * Add Category Default to details panel * Fix stockItem query count * Fix reviewed issues * . * . * . * Prevent root category from checking parent for default location commit 6abd33f Author: Oliver <oliver.henry.walters@gmail.com> Date: Fri Mar 15 00:24:48 2024 +1100 Report enhancements (inventree#6714) * Add "enabled" filter to template table * Cleanup * API endpoints - Add API endpoints for report snippet - List endpoint - Details endpoint * Update serializers - Add asset serializer - Update * Check for duplicate asset files - Prevent upload of duplicate asset files - Allow re-upload for same PK * Duplicate checks for ReportSnippet * Bump API version commit cbd94fc Author: Oliver <oliver.henry.walters@gmail.com> Date: Thu Mar 14 23:06:11 2024 +1100 Fix for caddyfile (inventree#6712) - Add "authorization" to Access-Control-Allow-Headers - CORS requests actually *work* now commit ec5ff64 Author: Lukas <76838159+wolflu05@users.noreply.github.com> Date: Thu Mar 14 13:03:30 2024 +0100 handle report previewing errors (inventree#6709) commit 267ff67 Author: Oliver <oliver.henry.walters@gmail.com> Date: Thu Mar 14 15:11:27 2024 +1100 [PUI] Updates (inventree#6707) * Add button to edit part category * Fix useMemo() * Edit stock location commit 610ea7b Author: Oliver <oliver.henry.walters@gmail.com> Date: Thu Mar 14 12:09:14 2024 +1100 Report: Add date rendering (inventree#6706) * Validate timezone in settings.py * Add helper functions for timezone information - Extract server timezone - Convert provided time to specified timezone * Add more unit tests * Remove debug print * Test fix * Add report helper tags - format_date - format_datetime - Update report templates - Unit tests * Add setting to control report errors - Only log errors to DB if setting is enabled * Update example report * Fixes for to_local_time * Update type hinting * Fix unit test typo commit 7de8738 Author: Oliver <oliver.henry.walters@gmail.com> Date: Wed Mar 13 21:37:56 2024 +1100 Update .env (inventree#6700) Fix comment - no need to change Caddyfile in most cases commit 2fef348 Author: Oliver <oliver.henry.walters@gmail.com> Date: Wed Mar 13 20:37:05 2024 +1100 Unit tests for HOST settings (inventree#6698) - CORS - ALLOWED_HOSTS

XanderLuciano · 2024-03-16T06:55:09Z

@wolflu05 @SchrodingersGat I've (mostly) fixed the issues with the original commit, and also improved the behavior in a few other places as well at the same time.

The modal forms will now shrink if the form is short, and I made the user groups display partially hidden into a spoiler so it should avoid the double scrollbar initially (some screen resolutions may still have double scroll bars).

I'm happy with how this behaves now though :) What do you guys think?

SchrodingersGat · 2024-03-16T22:26:23Z

@XanderLuciano looking pretty nice :) I would still like to see the "loading" overlay fixed, so there is not a large gap in the middle of the modal form initially

XanderLuciano · 2024-03-19T05:12:54Z

@SchrodingersGat I'm unable to reproduce the issue mentioned about the modal appearing as a large empty block upon first load (I simulated by throttling the network connection in dev tools). The modal is small until the API request for the fields list completes.

Unless I am misunderstanding and the issue is the desire is to have a loading overlay appear while the API request for fields is in progress?

SchrodingersGat · 2024-03-19T21:55:56Z

@XanderLuciano sorry I realise that the way I described it was misleading.

The modal is small until the API request for the fields list completes.

Correct, this is a much better description than mine!

Unless I am misunderstanding and the issue is the desire is to have a loading overlay appear while the API request for fields is in progress?

This is what I was looking for. Do you think you can tack it on to this PR? No worries if not, we can make it a separate thing

XanderLuciano · 2024-03-20T02:23:24Z

Shouldn't be too much trouble, and at this point I'm quite familiar with this single component so let's include it in the scope of this PR 😄 I'll mess around with some ideas tonight!

XanderLuciano · 2024-03-20T07:21:12Z

src/frontend/src/tables/settings/UserTable.tsx

+import {
+  useCreateApiFormModal,
+  useDeleteApiFormModal
+} from '../../hooks/UseForm';


Converted from old openCreateApiForm and openDeleteApiForm to newer useCreateApiFormModal and useDeleteApiFormModal

I believe this was leftover from the improvements done in #5897

This fixes an issue with the create new user button and the create new group button having different behavior on the admin page.

XanderLuciano · 2024-03-20T08:46:08Z

@SchrodingersGat Success! 😄

There was some unexpected issues with how we pre-specify fields to the model component that made this a bit trickier... Either we should hard code field values into the front end forms correctly, or not specify a fields object. For now, I changed the modals so they just disregard the field values because all the modals I tested had invalid field definitions being passed to them.

The end result, is a beautiful loading icon instead of an ugly blocky, broken looking form.

I think I'm happy to call this task completed if you're happy with the behavior as well.

matmair

Lgtm

matmair · 2024-03-20T08:51:33Z

src/frontend/src/components/forms/ApiForm.tsx

@@ -139,6 +139,12 @@ export function OptionsApiForm({
  const formProps: ApiFormProps = useMemo(() => {
    const _props = { ...props };

+    // This forcefully overrides initial data
+    // Currently, most modals do not get pre-loaded correctly


@XanderLuciano what do you mean by that? Do you have a sample?

If you look at the difference in the values for the fields object, there is a note able difference, and some of that data is required for the ApiFormField component to correctly appear, otherwise it shows the error message in the first photo. After the API call is made to the endpoint using OPTIONS and gets all the values for each field, it renders correctly / normally.

I think it's primarily missing the type attribute, which would need to be added everywhere a model form is called.

e.g. here

but that I think should be scoped out to a new issue.

The idea behind that is that one can define which fields to show and which to hide. Only fields added through fieldname: { /* ... */} should be shown. And then one could add some customization to the defaults requested via the OPTIONS request. E.g. hook up a value state or make the disabled state react to some other condition.

Does this still work?

@wolflu05 oh! Huh, surprisingly it does actually still work! Here is me removing the first name field on the edit user form

ok, got it, you only override if data is not defined. I now also tested if the reactiveness (hooking up a state and use that for the disabled state (see playground)) still works, seems so.

Yes! I re-used the idea of not rendering the entire ApiForm component if data was not available. Instead of not rending the component, I just override the fields temporarily until the data arrives.

Now that I know the intended behavior, the method I chose actually seems quite fitting IMO.

Accidentally worked just right 😄 that's rare!

@XanderLuciano @wolflu05 after testing the new code a bit more, I have found that this change breaks loading an "edit" form. The "initialData" query often returns first, before the "OPTIONS" (which tells us which fields are available).

Commenting out this line: _props.fields = undefined seems to result in correct behaviour again.

We need to review this ASAP as edit forms are really broken now

src/frontend/src/components/forms/ApiForm.tsx

SchrodingersGat · 2024-03-20T09:21:06Z

@XanderLuciano nice work the loader looks really good!

@wolflu05 are you also happy to merge this in?

wolflu05 · 2024-03-20T09:34:00Z

Yes, LGTM too.

SchrodingersGat · 2024-03-20T10:00:31Z

@XanderLuciano great work here! Sorry for the complications, but this has turned out very nicely indeed.

XanderLuciano · 2024-03-20T19:40:58Z

Thank you! I appreciate the quick response and feedback along the way.

SchrodingersGat · 2024-03-21T10:51:52Z

@XanderLuciano please see my inline review comment above - this PR has introduced an unfortunate new behaviour

- Ref: inventree#6699

* Fix for initial form data - Ref: #6699 * Hide fields until OPTIONS request is complete * Add divider at bottom of form * Fix forms.tsx

Quick attempt at fixed form footer

3c7e16c

XanderLuciano requested a review from SchrodingersGat as a code owner March 13, 2024 09:44

slightly improve on lower res devices

ce0291f

matmair assigned XanderLuciano Mar 13, 2024

matmair added the Platform UI Related to the React based User Interface label Mar 13, 2024

matmair added this to the 0.15.0 milestone Mar 13, 2024

matmair approved these changes Mar 13, 2024

View reviewed changes

XanderLuciano added 4 commits March 16, 2024 03:06

Merge branch 'master' into fix-modal-scroll

5cdce0c

Make ApiForms shrinkable, spoiler long group list

0d42d02

Improve API Form Scroll Behavior

b53d0fd

XanderLuciano requested a review from matmair March 16, 2024 06:58

Merge branch 'inventree:master' into fix-modal-scroll

c2dec38

Fix incorrect modal component

6e7838b

XanderLuciano commented Mar 20, 2024

View reviewed changes

XanderLuciano added 2 commits March 20, 2024 08:19

Force load all modal fields to trigger loading animation

d74c1c7

Show loading overlay while fetching fields

007b888

matmair approved these changes Mar 20, 2024

View reviewed changes

SchrodingersGat merged commit 97ec4d0 into inventree:master Mar 20, 2024
22 checks passed

XanderLuciano deleted the fix-modal-scroll branch March 20, 2024 19:41

SchrodingersGat added a commit to SchrodingersGat/InvenTree that referenced this pull request Mar 21, 2024

Fix for initial form data

26c8491

- Ref: inventree#6699

SchrodingersGat mentioned this pull request Mar 21, 2024

Fix for initial form data #6791

Merged

SchrodingersGat added a commit that referenced this pull request Mar 24, 2024

Fix for initial form data (#6791)

405ca18

* Fix for initial form data - Ref: #6699 * Hide fields until OPTIONS request is complete * Add divider at bottom of form * Fix forms.tsx

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make modal footer fixed (#6696) #6699

Make modal footer fixed (#6696) #6699

XanderLuciano commented Mar 13, 2024 •

edited by matmair

netlify bot commented Mar 13, 2024 •

edited

XanderLuciano commented Mar 13, 2024

SchrodingersGat commented Mar 13, 2024

wolflu05 commented Mar 13, 2024

matmair left a comment

SchrodingersGat commented Mar 13, 2024

SchrodingersGat commented Mar 14, 2024

XanderLuciano commented Mar 14, 2024

XanderLuciano commented Mar 16, 2024 •

edited

SchrodingersGat commented Mar 16, 2024

XanderLuciano commented Mar 19, 2024

SchrodingersGat commented Mar 19, 2024

XanderLuciano commented Mar 20, 2024

XanderLuciano Mar 20, 2024

XanderLuciano commented Mar 20, 2024

matmair left a comment

matmair Mar 20, 2024

XanderLuciano Mar 20, 2024

XanderLuciano Mar 20, 2024

wolflu05 Mar 20, 2024 •

edited

XanderLuciano Mar 20, 2024

wolflu05 Mar 20, 2024

XanderLuciano Mar 20, 2024

SchrodingersGat Mar 21, 2024

SchrodingersGat commented Mar 20, 2024

wolflu05 commented Mar 20, 2024

SchrodingersGat commented Mar 20, 2024

XanderLuciano commented Mar 20, 2024

SchrodingersGat commented Mar 21, 2024

Make modal footer fixed (#6696) #6699

Make modal footer fixed (#6696) #6699

Conversation

XanderLuciano commented Mar 13, 2024 • edited by matmair

netlify bot commented Mar 13, 2024 • edited

✅ Deploy Preview for inventree-web-pui-preview ready!

XanderLuciano commented Mar 13, 2024

SchrodingersGat commented Mar 13, 2024

wolflu05 commented Mar 13, 2024

matmair left a comment

Choose a reason for hiding this comment

SchrodingersGat commented Mar 13, 2024

SchrodingersGat commented Mar 14, 2024

XanderLuciano commented Mar 14, 2024

XanderLuciano commented Mar 16, 2024 • edited

SchrodingersGat commented Mar 16, 2024

XanderLuciano commented Mar 19, 2024

SchrodingersGat commented Mar 19, 2024

XanderLuciano commented Mar 20, 2024

Choose a reason for hiding this comment

XanderLuciano commented Mar 20, 2024

matmair left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wolflu05 Mar 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchrodingersGat commented Mar 20, 2024

wolflu05 commented Mar 20, 2024

SchrodingersGat commented Mar 20, 2024

XanderLuciano commented Mar 20, 2024

SchrodingersGat commented Mar 21, 2024

XanderLuciano commented Mar 13, 2024 •

edited by matmair

netlify bot commented Mar 13, 2024 •

edited

XanderLuciano commented Mar 16, 2024 •

edited

wolflu05 Mar 20, 2024 •

edited