-
Notifications
You must be signed in to change notification settings - Fork 180
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
feat: wip tooltip #72
Conversation
61bcba4
to
219d86a
Compare
219d86a
to
5cc0ff0
Compare
5cc0ff0
to
b57b376
Compare
b57b376
to
766855e
Compare
7682de5
to
cc33a75
Compare
'data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 ' + | ||
'data-[side=below]:slide-in-from-top-2 data-[side=above]:slide-in-from-bottom-2 ' + | ||
'data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 ' + | ||
'data-[side=after]:slide-in-from-left-2 data-[side=before]:slide-in-from-right-2 '; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this behave in a right-to-left layout?
Is this handled by tailwind or is it sliding from the "wrong" side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have a good point here too. I'll need to address this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elite-benni if you have time to tackle this feel free to create a PR. I am a little tied up right now and won't get to it in the next couple weeks...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added an PR, just removing the todo and data-[side=after]
classes.
First I thought I just have to add rtl:
selector for this.
As I tried out the solution i found out, that it is actually not a problem.
before
and after
are translated to left
and right
with taking the rtl direction into account.
Everything was fine. ;)
inputs: [ | ||
'brnTooltip: hlmTooltip', | ||
'aria-describedby', | ||
'disabled', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean, that we are not able to disable the tooltip without disabling the host?
I saw that you changed the input to only disabled
in the brn in comparison to matTooltipDisabled
.
How will we handle this, if we do not provide a seperate input for disabling only the tooltip?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. I'll change this in a follow up PR!
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/goetzrobin/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
What is the current behavior?
Closes #
What is the new behavior?
Does this PR introduce a breaking change?
Other information