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

Make moving items with images less buggy #1283

Merged
merged 2 commits into from
Oct 5, 2020

Conversation

OZZlE
Copy link
Contributor

@OZZlE OZZlE commented Sep 9, 2020

First of all, thank you a lot for an awesome module!

I have only one proposed change :-) When you try to drag on an image inside the grid for example then browsers usually starts dragging the image, like if you want to drag it into another application and save it there etc. This makes the actual dragging of items in the grid very buggy. At least half of the time when I tried to drag on top of an image in Chrome on my Mac it started dragging the image only + then I let go of mousedown and the item starts moving, but it also makes onLayoutChange not even fire.

So adding this simple css solves all that :-) Also fixes problems that might arise when dragging on text.

It took me quite a long while to realise this was the issue and I was going to create an issue on this module that onLayoutChange doesn't always fire even when you have moved something and it has fixated in position. But this small css fixes all that from what I've seen, sometimes life is actually simple :-)

Let me know what you think! :)

Issue: #1282

First of all, thank you a lot for an awesome module!

I have only one proposed change :-) When you try to drag on an image inside the grid for example then browsers usually starts dragging the image, like if you want to drag it into another application and save it there etc. This makes the actual dragging of items in the grid very buggy. At least half of the time when I tried to drag on top of an image in Chrome on my Mac it started dragging the image only + then I let go of mousedown and the item starts moving, but it also makes `onLayoutChange` not even fire.

So adding this simple css solves all that :-) Also fixes problems that might arise when dragging on text. 

It took me quite a long while to realise this was the issue and I was going to create an issue on this module that `onLayoutChange` doesn't always fire even when you have moved something and it has fixated in position. But this small css fixes all that from what I've seen, sometimes life is actually simple :-) 

Let me know what you think! :)
@STRML
Copy link
Collaborator

STRML commented Sep 10, 2020

Hi @OZZlE. Thanks for this PR. Unfortunately I don't think this is generalizable to all use cases of RGL - we have significant numbers of users who embed forms or charts or other interactive widgets inside their grid items. This CSS would cause problems with those use cases. Instead, it makes more sense to add a note about this to the README, and let users sort out what CSS should be added to their grid items depending on what they are doing with it.

@OZZlE
Copy link
Contributor Author

OZZlE commented Sep 14, 2020

@STRML aha, what do you think about adding it by default on only Images instead?

.react-grid-item img {
    pointer-events: none;
}

I have worked with web for 10 years, mostly FE, and I had no idea about this 'pointer-events' thing, I though this was a bug in this module and was very close to just opening an issue :-)

But yeah in readme could work otherwise as well :)

@STRML
Copy link
Collaborator

STRML commented Oct 5, 2020

I think the img styles are fine, yes - thanks for the suggestion!

@STRML
Copy link
Collaborator

STRML commented Oct 5, 2020

Speedy - nice!

@STRML STRML merged commit e4e2b8b into react-grid-layout:master Oct 5, 2020
@OZZlE OZZlE changed the title Make moving items with images and text less buggy Make moving items with images less buggy Oct 5, 2020
@OZZlE OZZlE deleted the patch-1 branch October 5, 2020 12:51
Heenawter added a commit to elastic/kibana that referenced this pull request Jan 9, 2023
)

## Summary

After upgrading to [React
17](#128239), the old version of
`react-grid-layout` was no longer compatible and required an upgrade as
well. However, this upgrade included a
[PR](react-grid-layout/react-grid-layout#1283)
that overcame a visual glitch with images, which made it so that a
clickable image in TSVB markdown was only clickable in a small region at
the bottom of the image.

This PR fixes this by undoing the changes made in the aforementioned
[PR](react-grid-layout/react-grid-layout#1283)
specifically for TSVB markdown images.

### Videos

**Before**


https://user-images.githubusercontent.com/8698078/208514574-e586c1e0-3675-4ef2-9a42-9956f81dee5a.mov


**After**


https://user-images.githubusercontent.com/8698078/208514585-353a0413-7ed2-40f9-8e90-8289b5483ecb.mov



### Checklist

- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 9, 2023
…tic#147802)

## Summary

After upgrading to [React
17](elastic#128239), the old version of
`react-grid-layout` was no longer compatible and required an upgrade as
well. However, this upgrade included a
[PR](react-grid-layout/react-grid-layout#1283)
that overcame a visual glitch with images, which made it so that a
clickable image in TSVB markdown was only clickable in a small region at
the bottom of the image.

This PR fixes this by undoing the changes made in the aforementioned
[PR](react-grid-layout/react-grid-layout#1283)
specifically for TSVB markdown images.

### Videos

**Before**

https://user-images.githubusercontent.com/8698078/208514574-e586c1e0-3675-4ef2-9a42-9956f81dee5a.mov

**After**

https://user-images.githubusercontent.com/8698078/208514585-353a0413-7ed2-40f9-8e90-8289b5483ecb.mov

### Checklist

- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit eaa9423)
kibanamachine referenced this pull request in elastic/kibana Jan 9, 2023
…#147802) (#148586)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Dashboard] Add styling to allow clickable TSVB markdown images
(#147802)](#147802)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Hannah
Mudge","email":"Heenawter@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-01-09T18:27:56Z","message":"[Dashboard]
Add styling to allow clickable TSVB markdown images (#147802)\n\n##
Summary\r\n\r\nAfter upgrading to
[React\r\n17](#128239), the old
version of\r\n`react-grid-layout` was no longer compatible and required
an upgrade as\r\nwell. However, this upgrade included
a\r\n[PR](https://github.com/react-grid-layout/react-grid-layout/pull/1283)\r\nthat
overcame a visual glitch with images, which made it so that
a\r\nclickable image in TSVB markdown was only clickable in a small
region at\r\nthe bottom of the image.\r\n\r\nThis PR fixes this by
undoing the changes made in the
aforementioned\r\n[PR](https://github.com/react-grid-layout/react-grid-layout/pull/1283)\r\nspecifically
for TSVB markdown images.\r\n\r\n###
Videos\r\n\r\n**Before**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/208514574-e586c1e0-3675-4ef2-9a42-9956f81dee5a.mov\r\n\r\n\r\n**After**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/208514585-353a0413-7ed2-40f9-8e90-8289b5483ecb.mov\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"eaa9423d51c043a9a6f4640cee26490b6d817dea","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Platform-Design","Feature:Dashboard","release_note:fix","Team:Presentation","loe:hours","Team:Visualizations","impact:low","backport:prev-minor","v8.7.0"],"number":147802,"url":"https://github.com/elastic/kibana/pull/147802","mergeCommit":{"message":"[Dashboard]
Add styling to allow clickable TSVB markdown images (#147802)\n\n##
Summary\r\n\r\nAfter upgrading to
[React\r\n17](#128239), the old
version of\r\n`react-grid-layout` was no longer compatible and required
an upgrade as\r\nwell. However, this upgrade included
a\r\n[PR](https://github.com/react-grid-layout/react-grid-layout/pull/1283)\r\nthat
overcame a visual glitch with images, which made it so that
a\r\nclickable image in TSVB markdown was only clickable in a small
region at\r\nthe bottom of the image.\r\n\r\nThis PR fixes this by
undoing the changes made in the
aforementioned\r\n[PR](https://github.com/react-grid-layout/react-grid-layout/pull/1283)\r\nspecifically
for TSVB markdown images.\r\n\r\n###
Videos\r\n\r\n**Before**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/208514574-e586c1e0-3675-4ef2-9a42-9956f81dee5a.mov\r\n\r\n\r\n**After**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/208514585-353a0413-7ed2-40f9-8e90-8289b5483ecb.mov\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"eaa9423d51c043a9a6f4640cee26490b6d817dea"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/147802","number":147802,"mergeCommit":{"message":"[Dashboard]
Add styling to allow clickable TSVB markdown images (#147802)\n\n##
Summary\r\n\r\nAfter upgrading to
[React\r\n17](#128239), the old
version of\r\n`react-grid-layout` was no longer compatible and required
an upgrade as\r\nwell. However, this upgrade included
a\r\n[PR](https://github.com/react-grid-layout/react-grid-layout/pull/1283)\r\nthat
overcame a visual glitch with images, which made it so that
a\r\nclickable image in TSVB markdown was only clickable in a small
region at\r\nthe bottom of the image.\r\n\r\nThis PR fixes this by
undoing the changes made in the
aforementioned\r\n[PR](https://github.com/react-grid-layout/react-grid-layout/pull/1283)\r\nspecifically
for TSVB markdown images.\r\n\r\n###
Videos\r\n\r\n**Before**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/208514574-e586c1e0-3675-4ef2-9a42-9956f81dee5a.mov\r\n\r\n\r\n**After**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/208514585-353a0413-7ed2-40f9-8e90-8289b5483ecb.mov\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"eaa9423d51c043a9a6f4640cee26490b6d817dea"}}]}]
BACKPORT-->

Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
…tic#147802)

## Summary

After upgrading to [React
17](elastic#128239), the old version of
`react-grid-layout` was no longer compatible and required an upgrade as
well. However, this upgrade included a
[PR](react-grid-layout/react-grid-layout#1283)
that overcame a visual glitch with images, which made it so that a
clickable image in TSVB markdown was only clickable in a small region at
the bottom of the image.

This PR fixes this by undoing the changes made in the aforementioned
[PR](react-grid-layout/react-grid-layout#1283)
specifically for TSVB markdown images.

### Videos

**Before**


https://user-images.githubusercontent.com/8698078/208514574-e586c1e0-3675-4ef2-9a42-9956f81dee5a.mov


**After**


https://user-images.githubusercontent.com/8698078/208514585-353a0413-7ed2-40f9-8e90-8289b5483ecb.mov



### Checklist

- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants