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

Tooltip visual refresh #2603

Merged
merged 30 commits into from
Mar 18, 2024
Merged

Tooltip visual refresh #2603

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6d55c3e
remove caret
langermank Feb 13, 2024
17f0ff4
bump pcss
langermank Feb 13, 2024
d6e3ff5
Create curly-camels-kneel.md
langermank Feb 13, 2024
b53679e
Update app/components/primer/alpha/tool_tip.ts
langermank Feb 13, 2024
d296ec1
Generating component snapshots
langermank Feb 13, 2024
5dadbc9
Update curly-camels-kneel.md
langermank Feb 13, 2024
709fa6f
Update .changeset/curly-camels-kneel.md
langermank Feb 13, 2024
a55b139
push for testing
langermank Feb 14, 2024
cdf6312
use css for offset
langermank Feb 14, 2024
0494b96
Generating component snapshots
langermank Feb 14, 2024
1c10803
add previews
langermank Feb 14, 2024
a5841c1
fix text selection on hover
langermank Feb 14, 2024
8544973
Merge branch 'toolti-visual-refresh' of https://github.com/primer/vie…
langermank Feb 14, 2024
a9348c3
Generating component snapshots
langermank Feb 14, 2024
74dcd5d
individual previews
langermank Feb 14, 2024
f3d2c0c
Merge branch 'toolti-visual-refresh' of https://github.com/primer/vie…
langermank Feb 14, 2024
5080635
Generating component snapshots
langermank Feb 14, 2024
6b9d301
Apply suggestions from code review
langermank Feb 14, 2024
c7b513d
Generating component snapshots
langermank Feb 14, 2024
1391f93
Update curly-camels-kneel.md
langermank Feb 14, 2024
6870f28
Merge branch 'main' into toolti-visual-refresh
langermank Feb 14, 2024
99983d3
Merge branch 'main' into toolti-visual-refresh
langermank Feb 21, 2024
477d187
fix alignment
langermank Feb 21, 2024
746d444
clean up snaps
langermank Feb 21, 2024
3ac149d
Generating component snapshots
langermank Feb 21, 2024
f98b218
Update curly-camels-kneel.md
langermank Feb 21, 2024
7d1bebd
Merging upstream
camertron Mar 18, 2024
6f654b6
Generating component snapshots
camertron Mar 18, 2024
f9f21be
Build
camertron Mar 18, 2024
921a5f2
Commit lockfile
camertron Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/curly-camels-kneel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
langermank marked this conversation as resolved.
Show resolved Hide resolved
---

Tooltip visual refresh to match Primer React
4 changes: 2 additions & 2 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ GEM
msgpack (1.7.2)
mutex_m (0.2.0)
nio4r (2.7.0)
nokogiri (1.15.5)
nokogiri (1.16.2)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
nokogiri (1.15.5-x86_64-linux)
nokogiri (1.16.2-x86_64-linux)
racc (~> 1.4)
octicons (19.8.0)
parallel (1.24.0)
Expand Down
76 changes: 2 additions & 74 deletions app/components/primer/alpha/tool_tip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
return (el: Element) => (selector ? el.matches(selector) : setSelector(el))
})()

const TOOLTIP_ARROW_EDGE_OFFSET = 6

Check failure on line 24 in app/components/primer/alpha/tool_tip.ts

View workflow job for this annotation

GitHub Actions / eslint 

'TOOLTIP_ARROW_EDGE_OFFSET' is assigned a value but never used

Check failure on line 24 in app/components/primer/alpha/tool_tip.ts

View workflow job for this annotation

GitHub Actions / eslint 

'TOOLTIP_ARROW_EDGE_OFFSET' is assigned a value but never used
langermank marked this conversation as resolved.
Show resolved Hide resolved
const TOOLTIP_SR_ONLY_CLASS = 'sr-only'
const TOOLTIP_OFFSET = 10
const TOOLTIP_OFFSET = 4

type Direction = 'n' | 's' | 'e' | 'w' | 'ne' | 'se' | 'nw' | 'sw'

Expand Down Expand Up @@ -92,15 +92,6 @@
text-wrap: balance;
}

:host:before{
position: absolute;
z-index: 1000001;
color: var(--bgColor-emphasis, var(--color-neutral-emphasis-plus));
content: "";
border: 6px solid transparent;
opacity: 0;
}

@keyframes tooltip-appear {
from {
opacity: 0;
Expand All @@ -110,15 +101,6 @@
}
}

:host:after{
position: absolute;
display: block;
right: 0;
left: 0;
height: 12px;
content: "";
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need this too keep the tooltip open when cursor leaves the trigger to hover the tooltip text.


:host(:popover-open),
:host(:popover-open):before {
animation-name: tooltip-appear;
Expand All @@ -127,65 +109,11 @@
animation-timing-function: ease-in;
}

:host(.\\:popover-open),
:host(.\\:popover-open):before {
:host(.\\:popover-open) {
animation-name: tooltip-appear;
animation-duration: .1s;
animation-fill-mode: forwards;
animation-timing-function: ease-in;
animation-delay: .4s;
}

:host(.tooltip-s):before,
:host(.tooltip-n):before {
right: 50%;
margin-right: -${TOOLTIP_ARROW_EDGE_OFFSET}px;
}
:host(.tooltip-s):before,
:host(.tooltip-se):before,
:host(.tooltip-sw):before {
bottom: 100%;
border-bottom-color: var(--bgColor-emphasis, var(--color-neutral-emphasis-plus));
}
:host(.tooltip-s):after,
:host(.tooltip-se):after,
:host(.tooltip-sw):after {
bottom: 100%
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these ::after classes are still needed to position the empty content that I mentioned here about hovering the tooltip text

:host(.tooltip-n):before,
:host(.tooltip-ne):before,
:host(.tooltip-nw):before {
top: 100%;
border-top-color: var(--bgColor-emphasis, var(--color-neutral-emphasis-plus));
}
:host(.tooltip-n):after,
:host(.tooltip-ne):after,
:host(.tooltip-nw):after {
top: 100%;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same https://github.com/primer/view_components/pull/2603/files#r1487031396

:host(.tooltip-se):before,
:host(.tooltip-ne):before {
left: 0;
margin-left: ${TOOLTIP_ARROW_EDGE_OFFSET}px;
}
:host(.tooltip-sw):before,
:host(.tooltip-nw):before {
right: 0;
margin-right: ${TOOLTIP_ARROW_EDGE_OFFSET}px;
}
:host(.tooltip-w):before {
top: 50%;
bottom: 50%;
left: 100%;
margin-top: -6px;
border-left-color: var(--bgColor-emphasis, var(--color-neutral-emphasis-plus));
}
:host(.tooltip-e):before {
top: 50%;
right: 100%;
bottom: 50%;
margin-top: -6px;
border-right-color: var(--bgColor-emphasis, var(--color-neutral-emphasis-plus));
}

@media (forced-colors: active) {
Expand Down
15 changes: 8 additions & 7 deletions demo/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions demo/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
"private": true,
"version": "0.1.0",
"dependencies": {
"@primer/css": "^21.1.1",
"@primer/css": "^21.2.0",
"@primer/primitives": "7.15.6",
"@rails/actioncable": "^7.1.3",
"@rails/ujs": "^7.1.3",
"turbolinks": "^5.2.0",
"webpack-dev-server": "^4.15.1"
}
}
}
Loading