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

[Fix] Fix and refactor Sorter.ts #6122

Closed
3 tasks done
mohamedsalem401 opened this issue Sep 4, 2024 · 2 comments
Closed
3 tasks done

[Fix] Fix and refactor Sorter.ts #6122

mohamedsalem401 opened this issue Sep 4, 2024 · 2 comments
Assignees

Comments

@mohamedsalem401
Copy link
Member

mohamedsalem401 commented Sep 4, 2024

  • Fix the Sorter usage in StyleManager (regression with sorting property layers)
  • Refactor Sorter with the usage of Components.canMove
  • Update canMove with the check of dropping main Symbol inside its own instance.
@artf artf added this to Roadmap Sep 2, 2024
@mohamedsalem401 mohamedsalem401 converted this from a draft issue Sep 4, 2024
@mohamedsalem401
Copy link
Member Author

The idea is to refactor the sorter for improved maintainability and readability. We can implement the following changes:

  1. Make Sorter class depend on an abstract tree structure rather than relying on specific implementations like Component or Layers, make the Sorter class work with a more generic, abstract tree structure. This will make it more flexible and reusable.
  2. Split the code to multiple smaller classes each with a single responsibility
  3. ComponentManager.canMove: Use the ComponentManager.canMove method to determine if a Component can be moved. This avoids duplicating code and ensures consistent behavior across different parts of your application.

We will see how we can apply those points one by one

  1. To make the Sorter class independant of any model implementation, we can add an argument in the SorterOptions that’s a function returning whether an element can be dragged into another element.

    export interface SorterOptions {
      container?: HTMLElement;
      canMove: (targetModel: Model<any>, srcModel: Model<any>, index: number) => Boolean;
      ...
    }

    For example the initializing the Sorter for the Layers in the StyleManager

    new utils.Sorter({
      container: this.el,
      canMove: (targetModel, srcModel, index) => {
    	  return targetModel.view === this.el // Allow dropping iff the target is the container
      },
      em,
    })

    And for anything depending on Component would be

    new utils.Sorter({
      container: this.el,
      canMove: editor.Components.canMove,
      em,
    })
  2. Splitting the functionality of Sorter class into multiple classes

    • DropLocationDeterminer
      • Receives input: Takes mouse movement data, the HTML element being dragged, the tree structure, and the canMove function as input.
      • Calculates drop location: Analyzes the mouse position, the structure of the tree, and the canMove function to determine the most suitable index for the dropped element.
    • DragAndDropHandler
      • Creates and manages elements: Creates and manages the placeholder, drag helper, and the actual element being dragged, ensuring their proper appearance and positioning.

@mohamedsalem401
Copy link
Member Author

This's done.

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Roadmap Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

1 participant