Fix visually cut off link search results in LinkControl #52618
Conversation
richtabor
added
[Type] Bug
|
Size Change: +28 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
|
Unfortunately, the Windows OS yields the opposite result. This PR seems to cut off the search results.
Probably because different operating systems apply different font families and have different element heights. on Windows OS, the height of the item in the search results is 49px. how about on Mac OS?
|
| @@ -118,7 +118,7 @@ $preview-image-height: 140px; | |||
| .block-editor-link-control__search-results { | |||
| margin-top: -$grid-unit-20; | |||
| padding: $grid-unit-10; | |||
| max-height: $grid-unit * 24; | |||
| max-height: $grid-unit * 24; // resolves to 194px | |||
There was a problem hiding this comment.
I think the question I have is why is it 194px? Why did we choose that value? If it's just "that's the visual max height we want" or "it allows for 4 search results" then let's include that here as it will help future devs 🙏
|
|
This component is a If the height of this button could be fixed, the problem might be solved. The following video shows that the height of this button also changes as the font family being rendered changes. 42e49f72edd38c230c5267135e3c9e5a.mp4 |
|
Perhaps it could be fixed height like buttons then? |
It may not be possible to use a simple fixed height. This is because in the current search results, long URLs will wrap and be higher than other buttons.
If we only consider the case where the first four search results all have URLs that do not wrap, we may be able to achieve our goal by ensuring a height that is independent of font family through min-height. Also, if it makes sense to truncate the URL, as in the title, a fixed height approach could be used.
|
I know these PRs. However, the Just as |
It does have a max width, though I suppose it could use an ellipsis truncation in case it goes further. Is that what you're referring to? |
Yes, to ensure that the URL with long slugs appears on a single line in search results, you might be able to make that change in another PR first and then come back to this PR. The CSS will probably look something like this: .block-editor-link-control__search-item .components-menu-item__info {
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
} |
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
|
Nothing that the overflow was addressed in #57775. I'll rebase this and try it again.
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
richtabor
force-pushed
the
fix/linkcontrol-results-height
branch
from
dd4996f
to
6464474
Compare
|
@t-hamano I rebased and updated, here's what I get with suggestions and very long titles/slugs. Mind checking on your end/reviewing?
|
|
Unfortunately, the result is exactly the same as reported in this comment 😅 Again, I think to resolve this issue on all OS, we need to define an explicit height for the button, as suggested in this comment. |
Does a 44px max-height on |
|
Flaky tests detected in 0cdae19. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7816500360
|
I took a look at this commit, but However, if the page has a long slug, it will overflow.
I think the solution is one of the following:
|
colorful-tones
added
the
Browser Issues
|
@t-hamano @richtabor How are we feeling about getting this ready for RC 1 next week? Seems like there are clear next steps. |
|
From my understanding, the current visual appearance differs depending on the OS, so strictly speaking, it's not a bug, but an improvement that eliminates OS differences. Additionally, this problem is not even a regression that happened in 6.5, so I think it can be removed from the project board (I have no idea why it was added to the project board 😅) |
Probably someone wanted to help shepherd this through. :)
I still consider this a bug, though it's not particularly immediate by any means. We know an appropriate fix to resolve it, we just need to optimize a bit and try it. There are plenty of similar experiences that don't have this occur. |
|
@fabiankaegy @annezazu From discussion above I think we can punt this to 6.6. |
|
With WordPress 6.6 Beta 1 targeted to be released in less than a day - Editor Triage Leads will move this to |
|
This is not a regression, so punting to 6.7 |
What?
Fixing a visual bug where link results were cut off visually due to the fixed height of the search results container.
How?
Adapts the height of the container so that the links display properly.
Testing Instructions
Screenshots or screencast