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

remove element centering #342

Merged
merged 2 commits into from
Feb 15, 2024
Merged

Conversation

loreanvictor
Copy link
Contributor

Checklist

  • I documented the TypeScript code using JSDoc style. (NA)
  • I added multiple screenshots/screencasts of my UI changes (NA)
  • I translated all the newly inserted strings into German and English (NA)

Motivation and Context

In Apollon, elements are centered, and their positions rounded to grids, when importing and exporting diagrams, and when diagrams are saved in any other storage (including local storage). This means what users see and what the actual diagram data are differ. For example the user might put two elements close to each other, but as a result of centering and rounding, they are not in fact as close as the user intends.

These issues specifically become highlighted in realtime collaboration. Patches emitted by Apollon need to be with respect to storable diagram data, as there are typically peers in a realtime collaboration network whose sole responsibility is to store the diagram, which would need to update the diagram accordingly matching the stored / exported format. Subsequently, when a user moves an element, in the perspective of realtime collaboration, all other elements are moved to. Or if a user refreshes their client, they will re-import the diagram with different coordinates than peers who have not refreshed, causing the clients to desync.

Description

This change removes aforementioned centering and rounding logic, and updates necessary test fixtures. Centering is important for SVG exports as they should not have empty spaces, but this is already fixed with previous fixes to SVG export bounding calculations.

Steps for Testing

  1. Run the client,
  2. Create elements and move them to some corner,
  3. Save the state, or export the diagram,
  4. Reload the state (refresh), or import the diagram,
  5. Elements remain exactly in the position they were saved / exported in.

Test Coverage

File Branch Line
components/store/model-store.tsx 100% 100%
components/store/model-state.ts 96.66% 95.11%

@loreanvictor loreanvictor marked this pull request as ready for review February 11, 2024 17:45
@matthiaslehnertum matthiaslehnertum self-requested a review February 15, 2024 20:31
@matthiaslehnertum matthiaslehnertum merged commit dcd4878 into develop Feb 15, 2024
5 checks passed
@matthiaslehnertum matthiaslehnertum deleted the bugfix/remove-element-centering branch February 15, 2024 20:32
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