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: remove extra spacing between 2 activity markers #3052

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Mar 6, 2024

When there are 2 markers, the center was translated/shifted too much. We assume that the width value of markers is SHAPE_ACTIVITY_MARKER_ICON_SIZE. We want to have a spacing value between the 2 markers of SHAPE_ACTIVITY_MARKER_ICON_MARGIN.

To have 2 markers horizontally centered and closed to each other, we need to translate the first marker to the left and the second to the right by half the value of the marker width. To add a space between the 2 markers, we need to translate both markers by half the value of the global "spacing value". Previously, the translation value included the full marker width which is too much.

Also include refactoring:

  • Use more accurate name for the parameters of getMarkerIconOriginFunction.

Notes

Covers #992
Detected as part of #2724

Additional information

The implementation to generalize markers positioning for 3 and more markers experimented in #2724 uses the same logic as the fix proposed here.

Here are explanations taken from #2724:

Previously, the translation includes SHAPE_ACTIVITY_MARKER_ICON_SIZE instead of SHAPE_ACTIVITY_MARKER_ICON_SIZE/2. The following schema explains why the value has to be divided by 2 👇🏿
⚠️ In the following schema

  • MARKER_WIDTH = SHAPE_ACTIVITY_MARKER_ICON_SIZE
  • MARKER_SPACING = SHAPE_ACTIVITY_MARKER_ICON_MARGIN

schema_bug_fix_marker-size_applied_twice_03

When there are 2 markers, the center was translated/shifted too much.
We assume that the width value of markers is `SHAPE_ACTIVITY_MARKER_ICON_SIZE`.
We want to have a spacing value between the 2 markers of `SHAPE_ACTIVITY_MARKER_ICON_MARGIN`.

To have 2 markers horizontally centered and closed to each other, we need to translate the first marker to the left and
the second to the right by half the value of the marker width.
To add a space between the 2 markers, we need to translate both markers by half the value of the global "spacing value".
Previously, the translation value included the full marker width which is too much.

Also include refactoring:
  - Use more accurate name for the parameters of `getMarkerIconOriginFunction`.
@tbouffard tbouffard added the bug Something isn't working label Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

♻️ PR Preview 14b68ef has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

Copy link

github-actions bot commented Mar 6, 2024

♻️ PR Preview 14b68ef has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

@tbouffard tbouffard marked this pull request as ready for review March 7, 2024 07:49
@tbouffard tbouffard requested a review from csouchet March 7, 2024 07:49
Copy link

sonarcloud bot commented Mar 7, 2024

@tbouffard tbouffard merged commit c0c50aa into master Mar 7, 2024
23 checks passed
@tbouffard tbouffard deleted the fix/extra_spacing_between_activity_markers branch March 7, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants