-
Notifications
You must be signed in to change notification settings - Fork 47
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
#4942 refactoring: Snackbar element #5534
base: angular_rework_development
Are you sure you want to change the base?
#4942 refactoring: Snackbar element #5534
Conversation
spbqaru
commented
Jun 11, 2024
•
edited
Loading
edited
- refactoring: Snackbar element
- Fix failing tests for Snackbar
- Update test site: Snackbar element
protected String messageLocator = "./span"; | ||
|
||
public class Snackbar extends UIBaseElement<SnackbarAssert> implements HasPosition { | ||
@Deprecated |
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.
пакет не релизился, ничего не надо depricated, просто удаляем
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.
fixed
@JDIAction("Click '{name}' action") | ||
public void clickAction() { | ||
action.click(); | ||
} | ||
|
||
@JDIAction("Click '{name}' action") |
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.
message не соответсвует действию
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.
fixed
@JDIAction("Click '{name}' action") | ||
public void clickAction() { | ||
action.click(); | ||
} | ||
|
||
@JDIAction("Click '{name}' action") | ||
public UIElement actionIcon() { | ||
return this.action; |
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.
а почему это icon?
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.
renamed
|
||
@Override | ||
public Position position() { | ||
return null; |
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.
странная позиция
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.
updated
* @param element UIElement to check | ||
* @return position as {@link Position} | ||
*/ | ||
default Position getPositionFromClass(UIElement element, String className) { |
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.
методы выглядят как статические, мы передаем элемент сами и класс тоже, никаких дефолтов даже нет
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.
overridden
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.
не понимаю смысла переопределения этого метода. У нас интерфейс, который говорит, что вот у этого элемента есть позиция, зачем вот этот метод мы должны переопределять, когда нас только текущая позиция интересует
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.
getPositionFromClass не надо переопределять,
getPositionFromAttribute - надо потому что берется позиция от родительского div
- Update test site: Snackbar element
- Update test site: Snackbar element
- Update test site: Snackbar element
- Update test site: Snackbar element
- Update test site: Snackbar element
jdi-light-angular/src/main/java/com/epam/jdi/light/angular/elements/complex/Snackbar.java
Outdated
Show resolved
Hide resolved
jdi-light-angular/src/main/java/com/epam/jdi/light/angular/elements/complex/Snackbar.java
Show resolved
Hide resolved
* @param element UIElement to check | ||
* @return position as {@link Position} | ||
*/ | ||
default Position getPositionFromClass(UIElement element, String className) { |
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.
не понимаю смысла переопределения этого метода. У нас интерфейс, который говорит, что вот у этого элемента есть позиция, зачем вот этот метод мы должны переопределять, когда нас только текущая позиция интересует
LEFT("justify-content: flex-start"), | ||
RIGHT("justify-content: flex-end"), | ||
|
||
CENTER_BOTTOM("justify-content: center; align-items: flex-end;"), |
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.
а если стили в другом порядке стоять будут, то все?
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.
обновил Position enum
- Update test site: Snackbar element
- Update test site: Snackbar element
- Update test site: Snackbar element
- Update test site: Snackbar element
- Update test site: Snackbar element
* Each constant includes information about its string representation. | ||
*/ | ||
public enum Position { | ||
TOP(null, "align-items: flex-start;"), |
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.
все зафейлится, если значение будет последним в стиле
return position; | ||
} | ||
} | ||
throw runtimeException(String.format("No appropriate %s constant found for value '%s'", Position.class.getName(), text)); |
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.
это потом везде ловить это исключение нужно, не удобно. лучше завести значение типа UNKNOWN и его возвращать, когда ничего не нашлось
if (StringUtils.isBlank(text)) { | ||
throw runtimeException(String.format("%s: input string can't be empty", Position.class.getName())); | ||
} | ||
boolean horizontalPositionMatches = containsIgnoreCase(text, justifyContent); |
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.
а почему нельзя взять css("justify-content") css("align-items") и их комбинировать и не парится, что там у нас в стиле с чем совпало?