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

allow more triangle styling and better placement #121

Conversation

malteludwig
Copy link
Contributor

@malteludwig malteludwig commented Mar 24, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • [ x] Feature
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • Documentation content changes

Other information

  • draw triangle using border-top to permit better size and placement handling.
  • add a drop-shadow to mimic your design (instead of border-box)
  • add vars allow styling from outside

Issue Number: N/A

@peterpeterparker peterpeterparker merged commit 74da29e into peterpeterparker:main Mar 25, 2023
@peterpeterparker
Copy link
Owner

Thanks for the PR 👍.

Had to make few adjustements after merge and I released it v0.0.44 afterwards.

@malteludwig
Copy link
Contributor Author

@peterpeterparker There is a case where the triangle is off the toolbar, when you select text at the very left:

Screenshot 2023-03-25 at 21 12 47

I had added this: left: max(var(--stylo-toolbar-triangle-start), calc(var(--stylo-triangle-width) / 2)); to prevent the triangle going to far to the left if --stylo-toolbar-triangle-star is negative

Screenshot 2023-03-25 at 21 32 16

@peterpeterparker
Copy link
Owner

@malteludwig thanks for the ping. this did not work if the position was set in percent - i.e. when the menu set 50% the triangle was not correct positioned neither.

I fixed this edge case in this commit d9b6696

Thanks for the ping

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