bug #155523 input fields will can't trigger draggable in popups by de…

[Cmoen11]

…fault

[coveralls]

Pull Request Test Coverage Report for Build 980

• 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 17.979%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/components/Content/Popup/Popup.js	0	1	0.0%

Totals
Change from base Build 976:	0.0%
Covered Lines:	161
Relevant Lines:	665

---

💛 - Coveralls

[Kashkovsky]

First of all, the build is red. You do not need any changes is elementsui-react in scope of this bug. Popup has default prop: handle: ".popup-container". But it doesn't match the latest Elements styling rules (we want to drag by header).
So the fix in Elements (CollectionField and LookupField) will look like the following:

[Kashkovsky]

Fix it in Elements and close this PR.

[Cmoen11]

First of all, the build is red. You do not need any changes is elementsui-react in scope of this bug. Popup has default prop: handle: ".popup-container". But it doesn't match the latest Elements styling rules (we want to drag by header).
So the fix in Elements (CollectionField and LookupField) will look like the following:

I'm not sure if I agree with you fully Denys. Because this repository should allow everyone to use our components. And I do not feel having the whole component draggable by default and prevent input fields to not be functional. I'll to the changes in Elements code, but still thinks that this pr should be implemented.

[Kashilas]

First of all, the build is red. You do not need any changes is elementsui-react in scope of this bug. Popup has default prop: handle: ".popup-container". But it doesn't match the latest Elements styling rules (we want to drag by header).
So the fix in Elements (CollectionField and LookupField) will look like the following:

I'm not sure if I agree with you fully Denys. Because this repository should allow everyone to use our components. And I do not feel having the whole component draggable by default and prevent input fields to not be functional. I'll to the changes in Elements code, but still thinks that this pr should be implemented.

In thread with this discussion - the Client team will most likely start using elementsui-react for some of their development, so I agree with Christian in disabling dragging by inputs in the default implementation here.

[Kashkovsky]

First of all, the build is red. You do not need any changes is elementsui-react in scope of this bug. Popup has default prop: handle: ".popup-container". But it doesn't match the latest Elements styling rules (we want to drag by header).
So the fix in Elements (CollectionField and LookupField) will look like the following:

I'm not sure if I agree with you fully Denys. Because this repository should allow everyone to use our components. And I do not feel having the whole component draggable by default and prevent input fields to not be functional. I'll to the changes in Elements code, but still thinks that this pr should be implemented.

In thread with this discussion - the Client team will most likely start using elementsui-react for some of their development, so I agree with Christian in disabling dragging by inputs in the default implementation here.

This PR indicates complete misunderstanding on how the component should be used. The component receives the id or class name of the DOM element which should act like a drag handle. Otherwise the whole component will be draggable, because Popup cannot read developer's thoughts and decide it by its own. So instead of providing the correct property value to the component in consumer code you introduce some sort of hack due to inability to read the propTypes documentation for some reason:
/** CSS selector for the HTML element serving as a handle of draggable popup. Default: ".popup-container" */
handle: PropTypes.string

Please be sure you completely understand the code base before taking the responsibility to approve pull requests to the code base which might be used across different projects.

[Cmoen11]

First of all, the build is red. You do not need any changes is elementsui-react in scope of this bug. Popup has default prop: handle: ".popup-container". But it doesn't match the latest Elements styling rules (we want to drag by header).
So the fix in Elements (CollectionField and LookupField) will look like the following:

I'm not sure if I agree with you fully Denys. Because this repository should allow everyone to use our components. And I do not feel having the whole component draggable by default and prevent input fields to not be functional. I'll to the changes in Elements code, but still thinks that this pr should be implemented.

In thread with this discussion - the Client team will most likely start using elementsui-react for some of their development, so I agree with Christian in disabling dragging by inputs in the default implementation here.

This PR indicates complete misunderstanding on how the component should be used. The component receives the id or class name of the DOM element which should act like a drag handle. Otherwise the whole component will be draggable, because Popup cannot read developer's thoughts and decide it by its own. So instead of providing the correct property value to the component in consumer code you introduce some sort of hack due to inability to read the propTypes documentation for some reason:
/** CSS selector for the HTML element serving as a handle of draggable popup. Default: ".popup-container" */
handle: PropTypes.string

Please be sure you completely understand the code base before taking the responsibility to approve pull requests to the code base which might be used across different projects.

Default the popup has this class, which is fine. You're still able to add input fields, this will not work due to the draggable event canceling the input. Should this REALLY be the default behavior? in which cases do you have draggable input-fields?

What if you don't want to have this draggable header, but still want to have input-fields inside of it? Shouldn't you be allowed? Please provide more example in the doc, this is clearly not documented enough.

[dmarcautan]

@Kashilas @Cmoen11 @Kashkovsky The problem here is that 'cancel' prop accepts only single selector. So you will still be able to drag by <textarea />, <select_ /> etc. with such setup. Instead I suggest mirroring 'cancel' prop to our Popup component and default it to some service css class value like 'no-drag'. You will have to mark some top-level html element in your popup container element with this class so that any inner inputs of any kind don't trigger dragging.

[dmarcautan]

To be clear. Changes suggested don't fix the bug. You have to fix it in the app using it (Elements). Since it is urgent I recommend fixing it by actually specifiyng the proper 'handle' selector in code using it. You will lose possibility to drag by any part of popup but it's quick and easy. But solution suggested here should be considered in future use.

[Kashilas]

Yep. Already present in PR for Elements :)
https://dev.azure.com/elements/Projects/_git/Elements/pullrequest/8648?_a=overview