Skip to content

fix(button): don't needlessly overwrite title #8418

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

Merged
merged 0 commits into from
Dec 14, 2023
Merged

fix(button): don't needlessly overwrite title #8418

merged 0 commits into from
Dec 14, 2023

Conversation

maxpatiiuk
Copy link
Member

@maxpatiiuk maxpatiiuk commented Dec 13, 2023

Related Issue: #8417

Summary

In my application I set title attribute on the <calcite-button>, but it wasn't being displayed because the shadow DOM's <button> had an empty title attribute (title="") which was overwriting the title set on the calcite-button.

Reproduction:

  1. Codepen example
  2. Hover over the button - no tooltip is shown
  3. Remove the "title" attribute from the <button>, and tooltip is visible

Video reproduction:

out.mp4

@maxpatiiuk maxpatiiuk self-assigned this Dec 13, 2023
@maxpatiiuk maxpatiiuk requested a review from a team as a code owner December 13, 2023 17:08
@benelan benelan merged commit c8835a6 into Esri:main Dec 14, 2023
@maxpatiiuk
Copy link
Member Author

@benelan there may be something wrong with the pull bot. both of my PRs are marked as "merged" (this one, and #8385), despite changes not being merged
the bot seems to force-push, which overwrites the commits I have

@benelan
Copy link
Member

benelan commented Dec 19, 2023

Sorry I thought I responded to this! I'm pretty sure this is something on your end. I don't think we can do anything to push push to your fork without you giving us write privilege. Did you install GitHub's Pull App? I think you can check in your account's Application settings.

@maxpatiiuk
Copy link
Member Author

You are correct - I forgot that I even had that enabled.
Thank you for the information)
Opened a new pull request - #8491

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