Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grid reordering with images is v. buggy #2500

Closed
liamcrean opened this issue Nov 7, 2022 · 18 comments · Fixed by #2589, #2651 or #2665
Closed

Grid reordering with images is v. buggy #2500

liamcrean opened this issue Nov 7, 2022 · 18 comments · Fixed by #2589, #2651 or #2665
Assignees

Comments

@liamcrean
Copy link

liamcrean commented Nov 7, 2022

When adding images to file grid, or grid fields and then trying to reorder the rows with drag behaviour the UI feedback is jittery. It's a bit of lottery whether or not the row dragged up will appear above the field (usually not, sometimes moving a few rows down even). It's not that great without images, but with, it feels like a safecracking level of sensitivity and the UI indicators for row placement are not clear at all.

This is especially apparent on grids with more rows than visible in the viewport (e.g. more than 5/6).

Other drag and drop behaviours in the control panel work well — Relationships, fluid fields or reordering the actual grid field type columns (admin view) for example.

Tested in Firefox (106.0.2) / Chrome (107.0.5304.87) — both Mac OS

@TomJaeger
Copy link
Contributor

Hi Liam,

Thanks for posting this. What version of ExpressionEngine is this on?

@liamcrean
Copy link
Author

liamcrean commented Nov 7, 2022

Hi Tom

Latest build: 7.1.6

Ta

@pyrobob83
Copy link

Happening for me too in 7.2.0 - specifically a File Grid field with just the image and one other text field for the alt text. It's pretty much unusable to reorder the rows in both Firefox and Chrome.

@liamcrean
Copy link
Author

liamcrean commented Dec 8, 2022

Any update on this?

I have a Pro build trying to manage a LOT of image galleries and it has basically rendered re-ordering all but useless for them (running EE7.2.3). Tested here Firefox/Chrome latest builds.

They're having to delete each set of grid entries and carefully pick each item one by one to ensure they're in the right order from the start. This seems to be more evident with images that are portrait - or a mix. Hard to tie it down exactly. But it's pretty buggy as it stands.

Thanks

EDIT: just seen pull request above. Not sure how I missed that... Looks like it's in hand?

@intoeetive
Copy link
Contributor

@liamcrean we'd be happy if you could check out that PR and confirm it's fixing the things for you (however it does not include compressed JS files, as those are generated only as part of release process... so if you don't have installation from repo for this site, you'd probably have to wait - but we'll try to release it ASAP)

@liamcrean
Copy link
Author

Thanks @intoeetive

The site it's most evident on is a production build and not installed/maintained via this repo — so will wait for general release on that build.

When I get a chance I'll install this as a dev build. See if I can test it.

Thanks

@liamcrean
Copy link
Author

liamcrean commented Dec 11, 2022

Deleted last bit of feedback. Wasn't on correct PR branch (oops).

This release looks to have solved the issue. Thanks 👍

(aside: The UI could still do with some TLC - but I'm not seeing the weird behaviours. Great work)

@intoeetive
Copy link
Contributor

Thank you for testing and confirmation @liamcrean !

matthewjohns0n added a commit that referenced this issue Dec 12, 2022
Resolved #2500 Where grid reordering with images is buggy
@TomJaeger
Copy link
Contributor

Quick heads up to everyone following this. Fix went live today.

@liamcrean
Copy link
Author

liamcrean commented Dec 15, 2022

Sadly this fix isn't working on production.

I tested the fix on a relatively small set of grid fields (maybe 4 or 5). As soon as I went to re-order a much larger grid (maybe 20-25 rows) the problem resurfaced. Anything above 7 is pretty much a lottery (when using portrait images).

Oddly the bottom of this grid re-orders without too much problem. So looks like it's got something to do with a row-height total variable not being stored correctly for selected row - or perhaps totals not being added / exponential? This would make sense why it affects portrait images more than landscape (double row height, relatively speaking).

@liamcrean liamcrean reopened this Dec 15, 2022
@Yulyaswan
Copy link
Collaborator

@liamcrean I'm sorry to hear this. Will check again with this specific condition (20-25 rows and using portrait images)

@pyrobob83
Copy link

I can confirm it's not fixed for me either in 7.2.4. In my case it doesn't even appear to be the landscape/portrait thing or the number of entries - I have a File Grid field with 6 rows, all with the exact same size image and I still can't use it - see attached.

ee7.2.4-file-grid-reorder-issue.mp4

@liamcrean
Copy link
Author

Yes. Appears that by about the 6th grid entry — to do with grid row heights. The taller the row, the less fields it takes to cause the issue.

@liamcrean
Copy link
Author

liamcrean commented Dec 21, 2022

Really sorry to report that there are still issues with this on larger grids (which was always the problem). Please test this on a grid/file grid with say 15-20 entries (including images).

I've tryed to explain - but the best way I can put it is, that the adjustment is cumularitve, and the more rows that are added the more issue is evident. So it seems that a variable total is leaking so that the drag spacing breaks.

(not sure how to reopen).

@Yulyaswan
Copy link
Collaborator

@liamcrean I would be appreciated if you can check the last PR. It should fix the problem

@pyrobob83
Copy link

pyrobob83 commented Dec 22, 2022

@Yulyaswan That file seems to be compiled before being added to the dist files - I don't have the /src/ folder in my builds. Is there another way we can test this?

UPDATE: Nevermind, I found it: /themes/ee/asset/javascript/compressed/cp/grid.js - and can confirm this fixes the problem for me! Thank you!

@liamcrean
Copy link
Author

Can cofirm — Working perfectly for for me in Firefox/Chrome in OSX (all latest). Ran the test on a staging build grid fields both with about 25 portrait images... Hurrah! Thanks @Yulyaswan 👍

I assume this is the same code used for file grid too?

@Yulyaswan
Copy link
Collaborator

yes, it works for file grid too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment