-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Site Editor: Navigation panel add blue dot to customized templates #26894
Conversation
Size Change: +301 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
} | ||
|
||
.edit-site-navigation-panel__template-item-customized-dot { | ||
background: #fff; |
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.
We could go with $white
since it is defined as this in _colors.scss
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.
Works as advertised!
I've left a couple of comments. Nothing major, but I'd like a second opinion from design. 🙂
.edit-site-navigation-panel__template-item-customized-dot { | ||
position: absolute; | ||
right: $grid-unit-05; | ||
top: 50%; | ||
margin-top: -$grid-unit-05; | ||
width: $grid-unit-10; | ||
height: $grid-unit-10; | ||
display: block; | ||
background: var(--wp-admin-theme-color); | ||
border-radius: 50%; | ||
} |
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 about inline block?
.edit-site-navigation-panel__template-item-customized-dot { | |
position: absolute; | |
right: $grid-unit-05; | |
top: 50%; | |
margin-top: -$grid-unit-05; | |
width: $grid-unit-10; | |
height: $grid-unit-10; | |
display: block; | |
background: var(--wp-admin-theme-color); | |
border-radius: 50%; | |
} | |
.edit-site-navigation-panel__template-item-customized-dot { | |
background: var(--wp-admin-theme-color); | |
border-radius: 50%; | |
display: inline-block; | |
height: $grid-unit-10; | |
margin-left: 4px; | |
width: $grid-unit-10; | |
} |
position="top center" | ||
> | ||
<span className="edit-site-navigation-panel__template-item-customized-dot" /> | ||
</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.
I wonder if the dot alone is just too small to be an effective hover area for the tooltip.
Maybe it could make sense to wrap the entire title? 🤔
Also the tooltip background seems to me === the sidebar background, making it hard to read.
@jameskoster do you have any opinions?
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.
Maybe it could make sense to wrap the entire title?
That would probably help with the criticism I had below. Its hard to get it to pop up or even know its there, especially with the cursor already being a pointer in the parent.
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.
This is working pretty well in my testing so far. Some behavioral nitpicking:
It feels odd having a tooltip available on hover with no change in cursor to help distinguish that. Normally it might go from 'defualt' to 'pointer' when mousing over a tooltip, but since we are using 'pointer' in the parent container itself there is nothing telling us that anything special is there. Combining that with the delay that happens before the tooltip actually pops up, this does feel a bit funky and makes the tooltip feel hard to find and feel a bit more hidden. Thoughts?
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.
At the moment it seems that from-scratch templates are also being marked with the blue dot.
In all other applications of the blue dot pattern, it indicates that something has been customised. I don't think it would make sense to deviate that here by highlighting the from-scratch templates, which of course haven't been customised :)
packages/edit-site/src/components/navigation-sidebar/navigation-panel/style.scss
Outdated
Show resolved
Hide resolved
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.
In all other applications of the blue dot pattern, it indicates that something has been customized. I don't think it would make sense to deviate that here by highlighting the from-scratch templates, which of course haven't been customized :)
It looks like this is now fixed after the updates from master. 👍 Or not, im confusing myself in testing.
packages/edit-site/src/components/navigation-sidebar/navigation-panel/style.scss
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/navigation-sidebar/navigation-panel/style.scss
Outdated
Show resolved
Hide resolved
Blocked until #27016 is merged |
00ec15d
to
66e0141
Compare
We decided to abandon this approach in #27041 (comment). |
Description
Resolves #26451
Add a blue dot to customized templates.
How has this been tested?
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: