Skip to content

fix: remove extra spacing between 2 activity markers#3052

Merged
tbouffard merged 2 commits into
masterfrom
fix/extra_spacing_between_activity_markers
Mar 7, 2024
Merged

fix: remove extra spacing between 2 activity markers#3052
tbouffard merged 2 commits into
masterfrom
fix/extra_spacing_between_activity_markers

Conversation

@tbouffard

@tbouffard tbouffard commented Mar 6, 2024

Copy link
Copy Markdown
Member

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
@github-actions

github-actions Bot commented Mar 6, 2024

Copy link
Copy Markdown

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

🤖 By surge-preview

@github-actions

github-actions Bot commented Mar 6, 2024

Copy link
Copy Markdown

♻️ 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
@sonarqubecloud

sonarqubecloud Bot commented Mar 7, 2024

Copy link
Copy Markdown

@tbouffard tbouffard merged commit c0c50aa into master Mar 7, 2024
@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