Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(panel): loosen up bounds assessment #10651

Merged
merged 1 commit into from
Jan 22, 2018
Merged

fix(panel): loosen up bounds assessment #10651

merged 1 commit into from
Jan 22, 2018

Conversation

oliversalzburg
Copy link
Contributor

@oliversalzburg oliversalzburg commented May 9, 2017

getBoundingClientRect() will return a width of 0 in certain scenarios.
Retrieving the width through clientWidth will hold the desired width for the position calculation.

As this can be reversed for other situations, we take the max of both values.

Closes #10543

getBoundingClientRect() will return a width of 0 in certain scenarios.
Retrieving the width through clientWidth will hold the desired width for
the position calculation.

As this can be reversed for other situations, we take the max of both
values.

Closes #10543
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label May 9, 2017
@oliversalzburg
Copy link
Contributor Author

The mdChips failure is already in master AFAIK. I'd be happy to wait and rebase later if needed.

@EladBezalel
Copy link
Member

@ErinCoughlan care to take a look?

Copy link
Contributor

@ErinCoughlan ErinCoughlan left a comment

Choose a reason for hiding this comment

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

LGTM

@DerekLouie I think this is the cause of the sliding animation bug as well.

@EladBezalel EladBezalel added the pr: merge ready This PR is ready for a caretaker to review label May 10, 2017
@fsmeier
Copy link
Contributor

fsmeier commented Jun 7, 2017

Any updates here @ThomasBurleson ?

@ThomasBurleson
Copy link
Contributor

@oliversalzburg - lgtm

@ThomasBurleson ThomasBurleson added needs: presubmit and removed pr: merge ready This PR is ready for a caretaker to review labels Jun 12, 2017
@ThomasBurleson ThomasBurleson added this to the 1.1.5 milestone Jun 12, 2017
@oliversalzburg
Copy link
Contributor Author

@ThomasBurleson Awesome :) What does the needs: presubmit tag refer to? Anything I need to take care of?

@ThomasBurleson
Copy link
Contributor

@oliversalzburg - No, that is a flag used to indicate readiness for strict internal testing; the work is on our end now.

@fsmeier
Copy link
Contributor

fsmeier commented Aug 22, 2017

@ThomasBurleson what is the timeline for the next version and the pull/merge requests? This request is there for 3 months already and not merged yet and no new release.
You even have persons who contribute like @oliversalzburg but yet his afford is just meaningless if it is not going to be released.

At our project we are still on 1.1.1 which also has exceptions for some components but updating is no choice since the last "bugfix" releases introduced new features and new bugs.

So pls.: focus on bugfixes, close every new feature and bring this project to a stable version for the "end".

Best,
Florian

@ThomasBurleson ThomasBurleson modified the milestones: 1.1.5, 1.1.6 Sep 5, 2017
@fsmeier
Copy link
Contributor

fsmeier commented Sep 5, 2017

@ThomasBurleson why is this now marked with 1.1.6 instead of 1.1.5? Without this fix tooltips are not usable in the described cases.

@fsmeier
Copy link
Contributor

fsmeier commented Sep 6, 2017

@ErinCoughlan @EladBezalel can you push it somehow forward to version 1.1.5? what is missing here to have it merged? with debugInfoEnabled(false) the tooltips are not usable so an update is not possible for us.

@EladBezalel
Copy link
Member

@IMM0rtalis as i said in - #9379 (comment)

@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed pr: needs presubmit labels Nov 21, 2017
@Splaktar Splaktar added presubmit-failures P3: important Important issues that really should be fixed when possible. severity: regression This issue is related to a regression labels Dec 4, 2017
@fsmeier
Copy link
Contributor

fsmeier commented Jan 3, 2018

@Splaktar Heyho, I wish you a happy new year!
So how is it continuing with this PR? What are the google internal failed tests? How can we finally merge this? What can we do to help?

Again another month passed without any change here. It is really frustrating. This PR is RDY since may!!! If every bugfix needs a year of merging-time we will be doomed! :(

@oliversalzburg
Copy link
Contributor Author

I'm assuming this means that Google is using angular-material is some of their own products and, with this change, there are issues in their product.

So there is really nothing to be done by outsiders, except waiting or forking. Let me know if you're forking! 😄

@Splaktar
Copy link
Member

Sorry for the delay. I am working to get more details on what kind of failures have been encountered. Obviously I can't just get access to the test results as there may be confidential information in them.

As @oliversalzburg mentioned, there is nothing that the community can do at this time. Google is very hesitant to make breaking changes to this library. It is in production in many internal and external applications around the world. These applications may depend upon the current behavior (even if erroneous).

For now, my focus is on getting all of the PRs merged that don't cause any breakages. Once those are all handled, I will push hard to get PRs merged that fix bugs and seem to be caught by tests that are too specific or false positives.

@Splaktar Splaktar assigned Splaktar and unassigned ThomasBurleson Jan 11, 2018
@Splaktar Splaktar added P2: required Issues that must be fixed. and removed P3: important Important issues that really should be fixed when possible. labels Jan 11, 2018
@Splaktar Splaktar modified the milestones: 1.1.6, 1.1.7 Jan 17, 2018
@Splaktar
Copy link
Member

@IMM0rtalis it looks like we've managed to get this through the presubmit phase. Hoping to be able to merge very soon.

@josephperrott josephperrott merged commit 27d0f7c into angular:master Jan 22, 2018
@oliversalzburg oliversalzburg deleted the fix/panel-positioning branch January 22, 2018 18:06
chmelevskij pushed a commit to chmelevskij/material that referenced this pull request Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P2: required Issues that must be fixed. pr: merge ready This PR is ready for a caretaker to review severity: regression This issue is related to a regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants