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

Correct search bar progress not filling-up toolbar height. Contributes TJC#212568 #138

Merged
merged 3 commits into from Aug 30, 2019

Conversation

dcaliste
Copy link
Contributor

As reported by @rozgwi on TJC, the search progress bar was not filling properly up the toolbar height. I'm also pushing an additional commit to correct the page indicator not highlighted on press.

@pvuorela and @jpetrell, during the redesigned, you choose to remove BackgroundItem from toolbar items. I guess it's by design to use only IconButton there. I've one question though: the search icon is still a BackgroundItem and when pressed, it is highlighted with a solid coloured rectangle, while all other icon buttons are not. Should we correct this inconsistency by making SearchBarItem to inherit IconButton instead of BackgroundItem ? Or should we add BackgroundItem to all other IconButtons ?

plugin/ToolBar.qml Outdated Show resolved Hide resolved
@pvuorela
Copy link
Contributor

For search item background, I'd maybe just remove it. Also to search opened state has strange fulll width background and icon colored.

@dcaliste
Copy link
Contributor Author

For search item background, I'd maybe just remove it.

Ok, done. At the beginning, I try to make SearchBarItem to inherit from IconButton, but since it has a lot more when expanded, I simply transform the BackgroundItem into a plain Item and adjusted a bit the signals and properties. I think, it's safer, not to introduce regression like that, than trying to make a IconButton being able to cope with the additional TextField and close icon when expanded.

@pvuorela
Copy link
Contributor

LGTM, thanks!

@pvuorela pvuorela merged commit be0af9f into sailfishos:master Aug 30, 2019
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.

None yet

2 participants