-
Notifications
You must be signed in to change notification settings - Fork 128
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
refactor: make classes for each move types #194
Conversation
614e203
to
a9043ec
Compare
src/states/DraggingState.ts
Outdated
const currentPanelPosition = currentPanel.getPosition(); | ||
const currentPanelSize = currentPanel.getSize(); | ||
|
||
let minimumDistanceToChange: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WoodNeck
Couldn't parts related to minimumDistanceToChange also enter the MoveType class area?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be done, but I also thought that the only changing part will be only for freeScroll
move type, so I might be hesitated.
But I guess you're right. This might be included on MoveType
I guess.
@@ -64,16 +65,36 @@ export interface FlickingOptions { | |||
moveType: MoveTypeOption; | |||
} | |||
|
|||
export type MoveTypeObjectOption = MoveTypeSnapOption | MoveTypeFreeScrollOption; | |||
export type MoveTypeStringOption = MoveTypeObjectOption["type"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WoodNeck wow~~ it's good
} = { | ||
SNAP: "snap", | ||
FREE_SCROLL: "freeScroll", | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WoodNeck What is this syntax?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By doing that, MOVE_TYPE.SNAP
's type inference will be "snap"
, while string
will be inferred if not.
9802137
to
6188b2c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue
#189, #190, #191
Details