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

Michele craft next #47

Open
wants to merge 199 commits into
base: master
Choose a base branch
from
Open

Michele craft next #47

wants to merge 199 commits into from

Conversation

mresposito
Copy link

New

Testing

  • Unit
  • Integration
  • Localhost

Screenshots

mresposito and others added 30 commits March 25, 2020 15:04
Caused by Subscriber, to be fixed
…s/craft.js into candulabs-michele-nits-to-project

# Conflicts:
#	lerna.json
#	packages/core/src/__tests__/Canvas.spec.tsx
Remove the need for side effects for changing layer name via the .setCustom action
Otherwise immer will mutate the object as we remove nodes
prevwong and others added 30 commits February 4, 2021 19:44
- Improves and simplifies EventHandlers internal API
- Removes hacky and unstable subscriber logic
This PR adds `draggedOver` to the `events`. It's almost the same as #37 but targets the `next` branch and the new EventHandlers, which also gives us a cleaner API. (#37 will be obsolete).

Since the new EventHandlers introduced a `Set` as the store for the EventHandlers, we can now store not only the id of the node we are dragging over, but all of its ancestors, too.

I also updated the Basic example with two different examples on how to make use of the new API.

Closes #28, #37, prevwong#183

---

The only complaint we have with the new API is the name `draggedOver`. @prevwong suggested to re-use the `hovered` event. If an element is being hovered, and the `dragged` event stores at least one nodeId, we are dragging over an element. I prefer a more explicit API over an implicit one. As a counter argument Prev suggested to make use of the `EventHelpers`. 

I, too am not a fan of the name `draggedOver` but lack the creativity to come up with a better name, but I prefer the DX of having an explicit event for dragging over elements. My argument is, that the developer could discover the API with just using code completion and without having to have a look at the documentation of `hovered` or looking at a guide / example.

Maybe @matdru,  @mresposito or @timc1 can chime in and discus the API. Not only these guys, but everyone is welcome to leave a comment. 👍
… the added node (prevwong#188)

* fixed lint errors

* added options parameter to be able to solve prevwong#152 in another PR

* fixed test

* changed type of required parameter
* add cypress

* add tests

* make basic example testable with testids

* update the cypress version

* add .DS_Store to root gitignore

* added new line

* moved drag and drop commands to separate file

* fix flaky test

Co-authored-by: Prev Wong <prevwong@gmail.com>
* fix: layers unsafe mutation

* chore: remove immer from core

* chore: enable immer plugins

* chore: handle deprecation

* fix: type

* chore: yarn-lock

* chore: type

* cleanup
* avoid applying the same actions twice

* fix bug in acitons
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.