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

Make positions possibly negatives #5690

Merged
merged 2 commits into from
May 31, 2024
Merged

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented May 31, 2024

Closes #5427

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

  • Updated RecordBoard.tsx to use getDraggedRecordPosition utility for calculating new positions, allowing negative values.
  • Added get-dragged-record-position.util.ts utility function to handle negative positions.
  • Added unit tests for getDraggedRecordPosition in get-dragged-record-position.util.test.ts.
  • Modified record-position.factory.ts to support negative positions and updated related tests.
  • Updated position.scalar.ts to validate and allow negative number positions.

Copy link
Contributor

@martmull martmull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thomtrp thomtrp merged commit fbd8714 into main May 31, 2024
14 checks passed
@thomtrp thomtrp deleted the tt-positions-can-be-negative branch May 31, 2024 12:17
Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @thomtrp left a small comment

recordAfterPosition?: number,
): number => {
if (isDefined(recordAfterPosition) && isDefined(recordBeforePosition)) {
return (recordBeforePosition + recordAfterPosition) / 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you return in your if, I think we can have something with less indentation and more readable like this. Wdyt?

export const getDraggedRecordPosition = (
  recordBeforePosition?: number,
  recordAfterPosition?: number,
): number => {
  if (isDefined(recordAfterPosition) && isDefined(recordBeforePosition)) {
    return (recordBeforePosition + recordAfterPosition) / 2;
  }

  if (isDefined(recordAfterPosition)) {
    return recordAfterPosition - 1;
  }

  if (isDefined(recordBeforePosition)) {
    return recordBeforePosition + 1;
  }

  return 1;
};

Weiko pushed a commit that referenced this pull request May 31, 2024
arnavsaxena17 pushed a commit to arnavsaxena17/twenty that referenced this pull request Oct 6, 2024
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.

Fix record position calculation
3 participants