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.

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

Remove padding in v-highlight to fix display issues #19740

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

paescuj
Copy link
Member

@paescuj paescuj commented Sep 19, 2023

Before

Before #19451

In #19451 an attempt was made to prevent the highlight background from being cut off (see left side):

After #19451

However, this led to an even stronger jump effect:

Although this jump effect could be prevented by always rendering text with v-highlight even if no search is applied (which feels like an unsafe requirement), there was still the problem that narrow letters are overlapped and the letters themselves are jumpy.

After

I've tried various approaches (absolute background behind the letter, ...), but the only one that works reliably is to leave out the padding of the background completely:

A small disadvantage is that the highlighting is a bit less visible now. On the other hand, it makes it clearer which part is actually highlighted - before that fix it was sometimes not obvious (for example here one could think that the "l" is also marked):

@changeset-bot
Copy link

changeset-bot bot commented Sep 19, 2023

🦋 Changeset detected

Latest commit: 422917a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@directus/app Patch
@directus/api Patch
directus Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@paescuj paescuj changed the title Remove padding in v-highlight to fix various display issues Remove padding in v-highlight to fix display issues Sep 20, 2023
@paescuj paescuj marked this pull request as ready for review September 20, 2023 09:54
@paescuj paescuj requested review from a team, rijkvanzanten, br41nslug and DanielBiegler and removed request for a team September 20, 2023 09:55
Copy link
Contributor

@DanielBiegler DanielBiegler left a comment

Choose a reason for hiding this comment

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

I checked the look for monospace-font:

Before After
image image

I really like this. Good job! ❤️

Copy link
Member

@br41nslug br41nslug left a comment

Choose a reason for hiding this comment

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

Looking much better on the data model page.

As a side note, the highlighting is not very visible using dark mode in the collection side bar
image

@br41nslug br41nslug merged commit 59f9ce4 into main Sep 20, 2023
6 checks passed
@br41nslug br41nslug deleted the fix-v-highlight-background branch September 20, 2023 11:46
@github-actions github-actions bot added this to the Next Release milestone Sep 20, 2023
br-rafaelbarros pushed a commit to personal-forks/directus-source that referenced this pull request Nov 7, 2023
* Remove padding in v-highlight to fix various display issues

* Add changeset
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants