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

fix(context-menu): updated context menu arrow left position calculation... #1268

Merged
merged 4 commits into from
Mar 5, 2019

Conversation

aefox
Copy link
Contributor

@aefox aefox commented Mar 1, 2019

to take into account IE limitations

Please read and mark the following check list before creating a pull request:

Short description of what this resolves:

According to #973 the white part of the arrow is misplaced on IE. That is because the left property value upon rendering becomes something like calc(50% - calc(11px - 2px)). IE does not support this style and the calculation is now changed to become calc(50% - (11px - 2px)).

Two extra things to note:

  • Initially I tried to make it be calc(50% - 11px - 2px) but for some very weird reason the result of that is incorrect (meaning the number of pixels is incorrect and the white part of the arrow is still misplaced
  • I tried for several hours to also re-align the menu items contents (as you can see in NbContextMenu in Internet Explorer (bad styling) #973 they are misaligned as well) but to no avail. Found the problem though: the span which contains the actual menu-item text is 0

@nnixaa
Copy link
Collaborator

nnixaa commented Mar 4, 2019

Hey @aefox, thanks for digging in! I had a quick look and it seems that the span is missing unfolded flex declaration flex: 1 0 auto. Could you try and update the PR?

@aefox
Copy link
Contributor Author

aefox commented Mar 5, 2019

@nnixaa brilliantly spot on! works like a charm on all browsers (IE 11, Edge, FF and Chrome)

@nnixaa nnixaa merged commit 0db8c2c into akveo:master Mar 5, 2019
@nnixaa
Copy link
Collaborator

nnixaa commented Mar 5, 2019

Great job @aefox! Looking forward to looking into your other PRs.

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.

2 participants