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

470 feature flag for grid workaround #473

Merged
merged 4 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 41 additions & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,25 @@ It applies globally (as in, all of your items everywhere are expected to have a

### Debug output

By default no debug output will be logged to the console. If you want to see internal debug messages, you can enable the debug output like this:
By default, no debug output will be logged to the console. If you want to see internal debug messages, you can enable the debug output like this:

```javascript
import {setDebugMode} from "svelte-dnd-action";
setDebugMode(true);
```

### Feature Flags

Feature flags allow controlling global optional behaviour. They were originally introduced as a way to enable a workaround for a browser bug that helps in certain scenarios but comes with unwanted side effects in others.
In order to set a feature flag use:

```javascript
import {setFeatueFlag, FEATURE_FLAG_NAMES} from "svelte-dnd-action";
setFeatueFlag(FEATURE_FLAG_NAMES.MY_FLAG, true);
```

Currently, there is only one flag: USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT, which defaults to false.
See issues [454](https://github.com/isaacHagoel/svelte-dnd-action/issues/454) and [470](https://github.com/isaacHagoel/svelte-dnd-action/issues/470) for details about why it is needed and when (most users don't need to care about this)

```javascript
import {setDebugMode} from "svelte-dnd-action";
Expand All @@ -233,28 +251,31 @@ setDebugMode(true);
### Typescript

If you are using Typescript, you will need to add the following block to your `global.d.ts` (at least until [this svelte issue](https://github.com/sveltejs/language-tools/issues/431) is resolved):

#### Svelte 3 or below

```typescript
declare type Item = import('svelte-dnd-action').Item;
declare type DndEvent<ItemType = Item> = import('svelte-dnd-action').DndEvent<ItemType>;
declare type Item = import("svelte-dnd-action").Item;
declare type DndEvent<ItemType = Item> = import("svelte-dnd-action").DndEvent<ItemType>;
declare namespace svelte.JSX {
interface HTMLAttributes<T> {
onconsider?: (event: CustomEvent<DndEvent<ItemType>> & { target: EventTarget & T }) => void;
onfinalize?: (event: CustomEvent<DndEvent<ItemType>> & { target: EventTarget & T }) => void;
}
interface HTMLAttributes<T> {
onconsider?: (event: CustomEvent<DndEvent<ItemType>> & {target: EventTarget & T}) => void;
onfinalize?: (event: CustomEvent<DndEvent<ItemType>> & {target: EventTarget & T}) => void;
}
}

```

#### Svelte 4:

```typescript
declare type Item = import('svelte-dnd-action').Item;
declare type DndEvent<ItemType = Item> = import('svelte-dnd-action').DndEvent<ItemType>;
declare type Item = import("svelte-dnd-action").Item;
declare type DndEvent<ItemType = Item> = import("svelte-dnd-action").DndEvent<ItemType>;
declare namespace svelteHTML {
interface HTMLAttributes<T> {
'on:consider'?: (event: CustomEvent<DndEvent<ItemType>> & { target: EventTarget & T }) => void;
'on:finalize'?: (event: CustomEvent<DndEvent<ItemType>> & { target: EventTarget & T }) => void;
}
}
interface HTMLAttributes<T> {
"on:consider"?: (event: CustomEvent<DndEvent<ItemType>> & {target: EventTarget & T}) => void;
"on:finalize"?: (event: CustomEvent<DndEvent<ItemType>> & {target: EventTarget & T}) => void;
}
}
```

You may need to edit `tsconfig.json` to include `global.d.ts` if it doesn't already: "include": ["src/**/*", "global.d.ts"].
Expand Down Expand Up @@ -318,15 +339,14 @@ You can use generics to set the type of `items` you are expecting in `DndEvent`.
items = e.detail.items;
}

let items: Dog[] = [
{ id: 1, name: 'Fido', breed: 'bulldog' },
{ id: 2, name: 'Spot', breed: 'labrador' },
{ id: 3, name: 'Jacky', breed: 'golden retriever' }
];
let items: Dog[] = [
{id: 1, name: "Fido", breed: "bulldog"},
{id: 2, name: "Spot", breed: "labrador"},
{id: 3, name: "Jacky", breed: "golden retriever"}
];
</script>
```


### Contributing [![contributions welcome](https://img.shields.io/badge/contributions-welcome-brightgreen.svg?style=flat)](https://github.com/isaacHagoel/svelte-dnd-action/issues)

There is still quite a lot to do. If you'd like to contribute please get in touch (raise an issue or comment on an existing one).
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"dist"
],
"description": "*An awesome drag and drop library for Svelte 3 (not using the browser's built-in dnd, thanks god): Rich animations, nested containers, touch support and more *",
"version": "0.9.24",
"version": "0.9.25",
"repository": {
"type": "git",
"url": "git+https://github.com/isaacHagoel/svelte-dnd-action.git"
Expand Down
8 changes: 7 additions & 1 deletion release-notes.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
## Svelte Dnd Action - Release Notes

### [0.9.25](https://github.com/isaacHagoel/svelte-dnd-action/pull/473)

Made the fix that was introduced in version 0.9.23 available via feature flag but inactive by default

### [0.9.24](https://github.com/isaacHagoel/svelte-dnd-action/pull/459)

Updated readme with Svelte 4 types configuration

### [0.9.23](https://github.com/isaacHagoel/svelte-dnd-action/pull/457)
Expand All @@ -8,7 +14,7 @@ Fix morphing when within css grid

### [0.9.22](https://github.com/isaacHagoel/svelte-dnd-action/pull/410)

Fix repl examples in Readme. Add svelte >=3.23.0 as peerDependency
Fix repl examples in Readme. Add svelte >=3.23.0 as peerDependency

### [0.9.21](https://github.com/isaacHagoel/svelte-dnd-action/pull/405)

Expand Down
7 changes: 6 additions & 1 deletion src/action.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,10 @@ function validateOptions(options) {
}

function isInt(value) {
return !isNaN(value) && (function(x) { return (x | 0) === x; })(parseFloat(value));
return (
!isNaN(value) &&
(function (x) {
return (x | 0) === x;
})(parseFloat(value))
);
}
2 changes: 1 addition & 1 deletion src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export let printDebug = () => {};

/**
* Allows the user to show/hide console debug output
* * @param {Boolean} isDebug
* * @param {boolean} isDebug
*/
export function setDebugMode(isDebug) {
if (isDebug) {
Expand Down
32 changes: 32 additions & 0 deletions src/featureFlags.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* @type {{USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT: string}}
*/
export const FEATURE_FLAG_NAMES = Object.freeze({
// This flag exists as a workaround for issue 454 (basically a browser bug) - seems like these rect values take time to update when in grid layout. Setting it to true can cause strange behaviour in the REPL for non-grid zones, see issue 470
USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT: "USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT"
});

const featureFlagsMap = {
[FEATURE_FLAG_NAMES.USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT]: false
};

/**
* @param {FEATURE_FLAG_NAMES} flagName
* @param {boolean} flagValue
*/
export function setFeatureFlag(flagName, flagValue) {
if (!FEATURE_FLAG_NAMES[flagName])
throw new Error(`Can't set non existing feature flag ${flagName}! Supported flags: ${Object.keys(FEATURE_FLAG_NAMES)}`);
featureFlagsMap[flagName] = !!flagValue;
}

/**
*
* @param {FEATURE_FLAG_NAMES} flagName
* @return {boolean}
*/
export function getFeatureFlag(flagName) {
if (!FEATURE_FLAG_NAMES[flagName])
throw new Error(`Can't get non existing feature flag ${flagName}! Supported flags: ${Object.keys(FEATURE_FLAG_NAMES)}`);
return featureFlagsMap[flagName];
}
11 changes: 6 additions & 5 deletions src/helpers/styler.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {SHADOW_ELEMENT_ATTRIBUTE_NAME, DRAGGED_ELEMENT_ID} from "../constants";
import {findCenter} from "./intersection";
import {svelteNodeClone} from "./svelteNodeClone";
import {getFeatureFlag, FEATURE_FLAG_NAMES} from "../featureFlags";

const TRANSITION_DURATION_SECONDS = 0.2;

Expand Down Expand Up @@ -77,9 +78,10 @@ export function morphDraggedElementToBeLike(draggedEl, copyFromEl, currentMouseX
left: (currentMouseX - draggedElRect.left) / draggedElRect.width,
top: (currentMouseY - draggedElRect.top) / draggedElRect.height
};
// The lines below are commented out because of issue 454 - seems like these rect values take time to update when in grid layout, therefore this gets copied from the computed styles now
// draggedEl.style.height = `${newRect.height}px`;
// draggedEl.style.width = `${newRect.width}px`;
if (!getFeatureFlag(FEATURE_FLAG_NAMES.USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT)) {
draggedEl.style.height = `${newRect.height}px`;
draggedEl.style.width = `${newRect.width}px`;
}
draggedEl.style.left = `${parseFloat(draggedEl.style.left) - relativeDistanceOfMousePointerFromDraggedSides.left * widthChange}px`;
draggedEl.style.top = `${parseFloat(draggedEl.style.top) - relativeDistanceOfMousePointerFromDraggedSides.top * heightChange}px`;
}
Expand Down Expand Up @@ -107,8 +109,7 @@ function copyStylesFromTo(copyFromEl, copyToEl) {
s === "color" ||
s === "list-style-type" ||
// copying with and height to make up for rect update timing issues in some browsers
s === "width" ||
s === "height"
(getFeatureFlag(FEATURE_FLAG_NAMES.USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT) && (s === "width" || s === "height"))
)
.forEach(s => copyToEl.style.setProperty(s, computedStyle.getPropertyValue(s), computedStyle.getPropertyPriority(s)));
}
Expand Down
44 changes: 22 additions & 22 deletions src/helpers/svelteNodeClone.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,33 @@
* Since svelte manages select value internally.
* @see https://github.com/sveltejs/svelte/issues/6717
* @see https://github.com/isaacHagoel/svelte-dnd-action/issues/306
*
* @param {HTMLElement} el
* @returns
*
* @param {HTMLElement} el
* @returns
*/
export function svelteNodeClone(el) {
const cloned = el.cloneNode(true);
const cloned = el.cloneNode(true);

const values = [];
const elIsSelect = el.tagName === "SELECT";
const selects = elIsSelect ? [el] : [...el.querySelectorAll('select')];
for (const select of selects) {
values.push(select.value);
}
const values = [];
const elIsSelect = el.tagName === "SELECT";
const selects = elIsSelect ? [el] : [...el.querySelectorAll("select")];
for (const select of selects) {
values.push(select.value);
}

if (selects.length <= 0) {
return cloned;
}
if (selects.length <= 0) {
return cloned;
}

const clonedSelects = elIsSelect ? [cloned] : [...cloned.querySelectorAll('select')];
for (let i = 0; i < clonedSelects.length; i++) {
const select = clonedSelects[i];
const value = values[i];
const optionEl = select.querySelector(`option[value="${value}"`);
if (optionEl) {
optionEl.setAttribute('selected', true);
const clonedSelects = elIsSelect ? [cloned] : [...cloned.querySelectorAll("select")];
for (let i = 0; i < clonedSelects.length; i++) {
const select = clonedSelects[i];
const value = values[i];
const optionEl = select.querySelector(`option[value="${value}"`);
if (optionEl) {
optionEl.setAttribute("selected", true);
}
}
}

return cloned;
return cloned;
}
2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ export {
overrideItemIdKeyNameBeforeInitialisingDndZones,
setDebugMode
} from "./constants";

export {setFeatureFlag, FEATURE_FLAG_NAMES} from "./featureFlags";
6 changes: 6 additions & 0 deletions typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,9 @@ export declare const DRAGGED_ELEMENT_ID: "dnd-action-dragged-el";
* Allows the user to show/hide console debug output
*/
export declare function setDebugMode(isDebug: boolean): void;

export enum FEATURE_FLAG_NAMES {
// Default value: false, This flag exists as a workaround for issue 454 (basically a browser bug) - seems like these rect values take time to update when in grid layout. Setting it to true can cause strange behaviour in the REPL for non-grid zones, see issue 470
USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT = "FEATURE_FLAG_NAMES.USE_COMPUTED_STYLE_INSTEAD_OF_BOUNDING_RECT"
}
export declare function setFeatureFlag(flagName: FEATURE_FLAG_NAMES, flagValue: boolean);