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.

Sign up for GitHub

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

Jump to bottom

Restore default slot and simple tip for Tooltip #7435

Closed
deboer-tim opened this issue Jun 3, 2024 · 0 comments · Fixed by #7436
Closed

Restore default slot and simple tip for Tooltip #7435

deboer-tim opened this issue Jun 3, 2024 · 0 comments · Fixed by #7436
Assignees
@deboer-tim
Labels
area/ui area/ui-components kind/enhancement ✨ Issue for requesting an improvement

Comments

@deboer-tim
Copy link
Collaborator

Is your enhancement related to a problem? Please describe

When I went to implement light mode on Tooltip (#6926) I saw that issue #7150 changed it so there are 2 slots (content and tip) and the bg and text colors are replicated on every single instance of Tooltip. We created the component to avoid duplicate code, so we should try to reduce this.

Describe the solution you'd like

Do a few things to make it easier to consume Tooltip:

  • Move 'content' back to be the default slot, since content must always be there and is naturally the visible child.
  • Make the outer box consistent - i.e. you are welcome to customize the content of tooltips, but the component creates the frame and bg color.
  • In additional to the 'tip' slot, allow 'tip' to be a property, so in the simple cases you can still do:
<Tooltip right tip="some tip">
  <something/>
</Tooltip>

and when you want to totally customize the tip:

<Tooltip right>
  <something/>
  <svelte:fragment slot="tip">tip content goes here</svelte:fragment>
</Tooltip>

I know the 'tip can either be property or slot' is a bit different (could use different names?), but I think this handles the simple/normal case the easiest while still allowing for customization.

Describe alternatives you've considered

No response

Additional context

No response
@deboer-tim deboer-tim added the kind/enhancement ✨ Issue for requesting an improvement label Jun 3, 2024
@deboer-tim deboer-tim self-assigned this Jun 3, 2024
deboer-tim added a commit to deboer-tim/desktop that referenced this issue Jun 3, 2024
@deboer-tim
chore: tooltip simplification
327adbc 
Changes
- Move 'content' back to be the default slot, since content must always be there and is naturally the visible child.
- Make the outer box consistent - i.e. you are welcome to customize the _content_ of tooltips, but the component creates the frame and bg color.
- In additional to the 'tip' slot, allow 'tip' to be a property, so in the simple cases you can still do:
```
<Tooltip right tip="some tip">
  <something/>
</Tooltip>
```
and when you want to totally customize the tip content:
```
<Tooltip right>
  <something/>
  <svelte:fragment slot="tip">tip content goes here</svelte:fragment>
</Tooltip>
```

If someone accidentally supplies both, the simple tip is used. Having two if-statements is minor duplication, but it is more obvious and avoids extra spaces in the html.

I know this undoes a bunch of change and 'tip can either be property or slot' is a bit different (could use different names?), but I think this handles the simple/normal case as easy as before and has a single 'frame', while still allowing for enough customization.

Fixes containers#7435.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
@deboer-tim deboer-tim mentioned this issue Jun 3, 2024
Merged
1 task
deboer-tim added a commit to deboer-tim/desktop that referenced this issue Jun 5, 2024
@deboer-tim
chore: tooltip simplification
939c386 
Changes
- Move 'content' back to be the default slot, since content must always be there and is naturally the visible child.
- Make the outer box consistent - i.e. you are welcome to customize the _content_ of tooltips, but the component creates the frame and bg color.
- In additional to the 'tip' slot, allow 'tip' to be a property, so in the simple cases you can still do:
```
<Tooltip right tip="some tip">
  <something/>
</Tooltip>
```
and when you want to totally customize the tip content:
```
<Tooltip right>
  <something/>
  <svelte:fragment slot="tip">tip content goes here</svelte:fragment>
</Tooltip>
```

If someone accidentally supplies both, the simple tip is used. Having two if-statements is minor duplication, but it is more obvious and avoids extra spaces in the html.

I know this undoes a bunch of change and 'tip can either be property or slot' is a bit different (could use different names?), but I think this handles the simple/normal case as easy as before and has a single 'frame', while still allowing for enough customization.

Fixes containers#7435.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
deboer-tim added a commit that referenced this issue Jun 6, 2024
@deboer-tim
chore: tooltip simplification
2f4b0ef 
Changes
- Move 'content' back to be the default slot, since content must always be there and is naturally the visible child.
- Make the outer box consistent - i.e. you are welcome to customize the _content_ of tooltips, but the component creates the frame and bg color.
- In additional to the 'tip' slot, allow 'tip' to be a property, so in the simple cases you can still do:
```
<Tooltip right tip="some tip">
  <something/>
</Tooltip>
```
and when you want to totally customize the tip content:
```
<Tooltip right>
  <something/>
  <svelte:fragment slot="tip">tip content goes here</svelte:fragment>
</Tooltip>
```

If someone accidentally supplies both, the simple tip is used. Having two if-statements is minor duplication, but it is more obvious and avoids extra spaces in the html.

I know this undoes a bunch of change and 'tip can either be property or slot' is a bit different (could use different names?), but I think this handles the simple/normal case as easy as before and has a single 'frame', while still allowing for enough customization.

Fixes #7435.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
deboer-tim added a commit to deboer-tim/desktop that referenced this issue Jun 6, 2024
@deboer-tim
chore: tooltip simplification
37cd3a2 
Changes
- Move 'content' back to be the default slot, since content must always be there and is naturally the visible child.
- Make the outer box consistent - i.e. you are welcome to customize the _content_ of tooltips, but the component creates the frame and bg color.
- In additional to the 'tip' slot, allow 'tip' to be a property, so in the simple cases you can still do:
```
<Tooltip right tip="some tip">
  <something/>
</Tooltip>
```
and when you want to totally customize the tip content:
```
<Tooltip right>
  <something/>
  <svelte:fragment slot="tip">tip content goes here</svelte:fragment>
</Tooltip>
```

If someone accidentally supplies both, the simple tip is used. Having two if-statements is minor duplication, but it is more obvious and avoids extra spaces in the html.

I know this undoes a bunch of change and 'tip can either be property or slot' is a bit different (could use different names?), but I think this handles the simple/normal case as easy as before and has a single 'frame', while still allowing for enough customization.

Fixes containers#7435.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
cdrage pushed a commit to cdrage/podman-desktop that referenced this issue Jun 19, 2024
@deboer-tim @cdrage
chore: tooltip simplification
39bb391 
Changes
- Move 'content' back to be the default slot, since content must always be there and is naturally the visible child.
- Make the outer box consistent - i.e. you are welcome to customize the _content_ of tooltips, but the component creates the frame and bg color.
- In additional to the 'tip' slot, allow 'tip' to be a property, so in the simple cases you can still do:
```
<Tooltip right tip="some tip">
  <something/>
</Tooltip>
```
and when you want to totally customize the tip content:
```
<Tooltip right>
  <something/>
  <svelte:fragment slot="tip">tip content goes here</svelte:fragment>
</Tooltip>
```

If someone accidentally supplies both, the simple tip is used. Having two if-statements is minor duplication, but it is more obvious and avoids extra spaces in the html.

I know this undoes a bunch of change and 'tip can either be property or slot' is a bit different (could use different names?), but I think this handles the simple/normal case as easy as before and has a single 'frame', while still allowing for enough customization.

Fixes containers#7435.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@deboer-tim deboer-tim

Labels
area/ui area/ui-components kind/enhancement ✨ Issue for requesting an improvement
Projects
Podman Desktop Planning
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: tooltip simplification deboer-tim/desktop
3 participants
@jeffmaury @deboer-tim @podman-desktop-bot