-
Notifications
You must be signed in to change notification settings - Fork 96
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
init #321
init #321
Conversation
src/etaoin/api.clj
Outdated
|
||
;; actions | ||
|
||
(defn get-id |
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.
лучше назвать rand-uuid, чтоб не путать с другими айдишниками
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.
ок
src/etaoin/api.clj
Outdated
|
||
(defn make-pointer | ||
[type] | ||
(assoc (make-action-input :pointer) :parameters {:pointerType 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.
здесь и далее: мне кажется, тут лучше через assoc-in
:
(-> (make-action-input :pointer)
(assoc-in [:parameters :pointerType] type))
потому что в первом случае, если словарь parameters
уже был, мы затрем его целиком, а так просто добавим ключ
src/etaoin/api.clj
Outdated
[type & [id]] | ||
{:type (name type) :id (or id (get-id)) :actions []}) | ||
|
||
(defn make-pointer |
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.
Предлагаю ко всем функциям, которые строят инпут, добавить -input
на конце, например make-mouse-input
и т.д. Потому что модуль api.clj
большой, и просто make-key
ничего не говорит.
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.
ок
src/etaoin/api.clj
Outdated
(defn make-pointer | ||
[type] | ||
(assoc (make-action-input :pointer) :parameters {:pointerType 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.
Тут скорее вопрос: базовый make-action-input
принимает необязательный id, а конечные make-mouse
и другие -- нет. Насколько важен этот айди? Спека говорит что-то о том, чтобы они были разные или наоборот, одинаковые?
Еще я заметил, что make-action-input
принимает id
остаточным аргументом, из-за чего его трудно передавать вниз. Може, сделать опциаональную мапу?
(defn make-action-input [type & [{:as opt :keys [id]}]])
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.
тогда этот opt
легко прокинуть из make-mouse и других функций. Только вопрос, нам это нужно?
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.
да, я его добавил и сам задумался насколько он нужен.
фактически этот id для драйвера, когда мы отправляем запрос первый раз он добавляет этот виртуальный девайс в таблицу девайсов(типо как сессия, только для девайса).
я бы наверно пока оставил чистый рандом, а если будет запрос чтобы в ручном режиме задавать, тогда можно будет вернутся к этому.
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.
Хорошо, в будущем можно будет добавить в эти функции опциональную мапу с айдишником и пробрасывать.
src/etaoin/api.clj
Outdated
(add-action input {:type "pointerMove" | ||
:x x | ||
:y y | ||
:origin origin |
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.
тут наверное добавить default-origin
по аналогии с duration
. B докстринге написать, какие значения может принимать origin
(со ссылкой на спеку).
Еще я спросил в задаче: в спеке сказано, что origin может быть объект, представляющий элемент. Я что-то припонимаю смутно, но не могу вспомнить -- как выразить такой элемент? Кажется, это словарь с каким-то особым ключом и значением из find-element
? Если да, по идее на место origin можно передать селектор, получить резултат и подать этот словарик на вход.
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.
Вспомнил! У нас для этого функция el->ref
. Стало быть, можно передать селектор, найти элемент, сделать ссылку и передать в origin.
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.
про origin ok, в задачке тоже отписал
да, можно так сделать.
я не все функции еще реализовал, в пн добавлю функций для мыши для перемещения к элементам и тп
src/etaoin/api.clj
Outdated
[input] | ||
(add-action input {:type "pointerCancel"})) | ||
|
||
(defmacro with-key [input key & body] |
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.
тут бы сделать понятней имя, например with-key-down
. По аналогии добавить with-pointer-down
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.
ок
src/etaoin/api.clj
Outdated
(add-pause) | ||
(add-pause) | ||
(add-pause) | ||
(add-pointer-move {:x (int x) :y (int y) :origin "viewport"}) |
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.
уточню, а зачем нужна конвертация в int? Без нее не работает?
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.
часто get-element-location
возвращает координаты с дробной частью, и если их не преобразовать в инты, драйвер возвращает эксепшн.
src/etaoin/api.clj
Outdated
|
||
|
||
(defn execute-actions | ||
[driver & actions] |
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.
Мне кажется, аргументы должны называться не actions
, а inputs
, потому что на верхнем уровне мы определяем инпуты, и полько под ними -- экшены.
И еще, чтобы не было резона сделать пустой вызов, можно сделать как минимум один инпут обязательными
[driver input & inputs]
...
{:actions (cons input inputs)}
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.
ок
src/etaoin/api.clj
Outdated
:data {:actions actions}})) | ||
|
||
|
||
(comment |
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.
просто не забыть, что это пойдет в тест. И можно ходить не в гугл, а на нашу страничку с текстовым вводом. Можно даже создать отдельную, чтобы проверить drag-n-drop.
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.
да, тоже думал локально сделать, в тестах у нас тоже drag-n-drop в сеть ходит
#103