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

The hover circle on material md-icon button is off when using fx-show / hide #105

Closed
geelus opened this issue Jan 9, 2017 · 14 comments · Fixed by #121
Closed

The hover circle on material md-icon button is off when using fx-show / hide #105

geelus opened this issue Jan 9, 2017 · 14 comments · Fixed by #121
Assignees
Labels
bug has pr A PR has been created to address this issue
Milestone

Comments

@geelus
Copy link

geelus commented Jan 9, 2017

When using fx-show / hide in a md-icon-button, the hover circle is a few pixels off. Here's a screenshot:
image

@ThomasBurleson
Copy link
Contributor

Issues are considered invalid without a working Plunkr or CodePen demo.


Also here are some important guidelines for investigating a Layout issue:

  • The Layout API is case-sensitive
    • fxlayout is not valid
    • fxLayout is valid
  • If you cannot get Flex-Layout to work with an example, try to manually use CSS flexbox stylings. If that still does not work to achieve your goals, then Flex-Layout is not the issue.
  • If hand-crafted CSS resolves the issue, but Flex-Layout does not then we definitely have a bug in this repository.

Templates

@geelus
Copy link
Author

geelus commented Jan 10, 2017

Here is the plunker:
https://plnkr.co/edit/P7Faj908RgiY84Ih3ivQ?p=preview

@geelus geelus changed the title The hover circle on material md-icon button is off when using fx-show / hide on toolbar The hover circle on material md-icon button is off when using fx-show / hide Jan 10, 2017
@ThomasBurleson
Copy link
Contributor

@geelus - wonderful. we will investigate.

@ThomasBurleson ThomasBurleson added this to the v2.0.0-rc.0 milestone Jan 10, 2017
@joshwiens
Copy link
Contributor

I actually just ran into this myself, looking into what's stepping on the circle now.

@geelus
Copy link
Author

geelus commented Jan 10, 2017

I fixed it by wrapping my button in a div and then applying the fxShow/fxHide to the div.

@joshwiens
Copy link
Contributor

@geelus - The difference between md-icon-button with / without fxShow is the height & width on md-button-wrapper

  • Without fxShow height & width are set to auto ( in the demo that's 40x40 ) and the icon is properly aligned.

  • With fxShow height & width are set to pixel values ( in the demo that's 23.9931x40 ) and the icon is left justified.

While wrapping the element in a <div> is a viable workaround, we shouldn't be stepping on element styles :)

@ThomasBurleson
Copy link
Contributor

@d3viant0ne - fxShow/fxHide simply set the display to none or flex.
I am not sure how that would change the px values from auto to px values.

@joshwiens
Copy link
Contributor

joshwiens commented Jan 11, 2017

@ThomasBurleson - It's fxShow changing display on md-icon-button from inline-block to flex that results in md-button-wrapper getting tweaked.

As you would expect, adding justify-content: center; to md-icon-button results in the proper alignment regardless of what width ends up being computed for md-button-wrapper.

By default, it's applying the default for justify-content thus we have a left aligned icon.

That brings up the question, is this really an issue with flex-layout or should md-icon-button be modified to properly justify it's icon? In the case of the icon button, I can't think of a reason why adding justify-content: center; would be benign for any other display value other then flex

Also worth noting the scope of this goes beyond just md-icon-button as initially reported. The above is applicable to any md-button.

//cc @jelbourn

@joshwiens joshwiens self-assigned this Jan 11, 2017
@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Jan 23, 2017

The fxHide and fxShow should not change the display mode to 'flex'. Instead the display mode (when not 'none') should be the default (=== 'block') or the explicitly assigned display mode for that element.

@see https://github.com/angular/flex-layout/pull/121/files#diff-3600d4c60f78022f7136ec3b187f680dR126

ThomasBurleson added a commit that referenced this issue Jan 23, 2017
The fxHide and fxShow should not change the display mode to 'flex'. Instead
the display mode (when not 'none') should be the default (=== 'block')
or the explicitly assigned display mode for that element.

Fixes #105.
@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Jan 23, 2017

@geelus, @d3viant0ne - see PR #121.

with these changes, the following works as expected:

<button md-icon-button [fxShow]="isVisible">
   <md-icon>menu</md-icon>
</button>

@ThomasBurleson ThomasBurleson added the has pr A PR has been created to address this issue label Jan 23, 2017
ThomasBurleson added a commit that referenced this issue Jan 23, 2017
The fxHide and fxShow should not change the display mode to 'flex'. Instead
the display mode (when not 'none') should be the default (=== 'block')
or the explicitly assigned display mode for that element.

Fixes #105.
@geelus
Copy link
Author

geelus commented Jan 23, 2017

@ThomasBurleson It doesn't seem to have been fixed in beta 3 or at least doesn't work for me. Can you please have this plunker working?
https://plnkr.co/edit/P7Faj908RgiY84Ih3ivQ?p=preview

@ThomasBurleson
Copy link
Contributor

@geelus - please note that the fixes are in a Pull Request that have NOT been merged nor released: #121

@geelus
Copy link
Author

geelus commented Jan 23, 2017

@ThomasBurleson Thanks for clarification. So I'll check next version (beta 4)

andrewseguin pushed a commit that referenced this issue Jan 24, 2017
* fix(fxHide,fxShow): fix standalone breakpoint selectors

* Many directive selectors were missing a `,` separator between then xs and gt-xs breakpoints
* fxShow, fxHide preserve and restore the element's original display CSS style
* added more tests standalone breakpoint selectors, disabled selectors, and deactivated mediaQueries

fixes #62, closes ##59.

* fix(api): add missing comma-delimiter in breakpoint selectors

* add test to use md breakpoint via `fxHide.md`

* fix(fxShow, fxHide): restore proper display mode when not active

The fxHide and fxShow should not change the display mode to 'flex'. Instead
the display mode (when not 'none') should be the default (=== 'block')
or the explicitly assigned display mode for that element.

Fixes #105.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug has pr A PR has been created to address this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants