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.

Sign up for GitHub

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

Jump to bottom

fix(ui5-button): now the hover state in the combi device changes color #4100

Merged
merged 6 commits into from
Oct 14, 2021
Merged

fix(ui5-button): now the hover state in the combi device changes color #4100

merged 6 commits into from
Oct 14, 2021

Conversation

Todor-ads
Copy link
Member

@Todor-ads Todor-ads commented Oct 8, 2021 •
edited
Loading

Fixes: #3783
Now the hover state in the combi device changes color

@Todor-ads Todor-ads changed the title fix(ui5-button): now the hover state in the combined device changes color fix(ui5-button): now the hover state in the combi device changes color Oct 8, 2021
@Todor-ads Todor-ads requested review from fifoosid and unazko October 8, 2021 13:36
unazko
unazko reviewed Oct 8, 2021
View reviewed changes
Copy link
Contributor

@unazko unazko left a comment

Choose a reason for hiding this comment

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

I've done manual testing on mobile device as well and it all looks good.
The build failed though may be re run the tests.

fifoosid
fifoosid suggested changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@fifoosid fifoosid left a comment

Choose a reason for hiding this comment

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

This change introduces regression. The hover of button on mobile is present again.
(The green button on the second line)
image

fifoosid
fifoosid suggested changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@fifoosid fifoosid left a comment

Choose a reason for hiding this comment

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

Present bugs:

On mobile(tested on IOS):

  • hover effect is still present

On Combi device(windows):

  • active state is missing

fifoosid
fifoosid reviewed Oct 11, 2021
View reviewed changes
Copy link
Contributor

@fifoosid fifoosid left a comment

Choose a reason for hiding this comment

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

You have to merge the master branch as there is merge conflict. You should rebuild the project, because the merge conflict is in the hash.txt file.

fifoosid
fifoosid reviewed Oct 14, 2021
View reviewed changes
Copy link
Contributor

@fifoosid fifoosid left a comment

Choose a reason for hiding this comment

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

You should merge the master branch again

@fifoosid fifoosid merged commit 6ab905c into master Oct 14, 2021
@fifoosid fifoosid deleted the button_lost_hover branch October 14, 2021 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unazko unazko unazko left review comments

@fifoosid fifoosid fifoosid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Buttons lost hover effect
3 participants
@Todor-ads @unazko @fifoosid