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

Autocompleter box size matches the content width #1175

Merged
merged 2 commits into from
Feb 3, 2025

Conversation

veloman-yunkan
Copy link
Collaborator

@veloman-yunkan veloman-yunkan commented Jan 27, 2025

Note however that no upper limit is set on the width of the autocompleter box - it is possible, but I don't see how we could come up with a good value for it.

Fixes #674

Note however that no upper limit is set on the width of the
autocompleter box - it is possible, but I don't see how we could
come up with a good value for it.
Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.47%. Comparing base (664944f) to head (2eea613).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1175   +/-   ##
=======================================
  Coverage   41.47%   41.47%           
=======================================
  Files          59       59           
  Lines        4272     4272           
  Branches     2336     2336           
=======================================
  Hits         1772     1772           
- Misses        999     1002    +3     
+ Partials     1501     1498    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kelson42
Copy link
Collaborator

kelson42 commented Jan 27, 2025

@veloman-yunkan I propose 300px (it's arbitrary), we should have ellipsis if this happens (title too long) and HTML hint, so if someone if hover its' still possible to see the full title. What do you think?

@veloman-yunkan
Copy link
Collaborator Author

Truncating the suggestion and adding a hint (to be displayed on hover) will be a lot more work. And it will be mostly unused on mobile. Should we really pursue it?

@kelson42
Copy link
Collaborator

@veloman-yunkan All of this should be doable via a few CSS directives... or do I get it wrong!

@veloman-yunkan
Copy link
Collaborator Author

@veloman-yunkan All of this should be doable via a few CSS directives... or do I get it wrong!

Indeed, text-overflow: ellipsis solves the part I was most concerned with.

@veloman-yunkan
Copy link
Collaborator Author

Done. Please test.

Note however that I don't like in this PR that autoComplete.css is modified. It may be better to move the CSS customization of autocomplete_wrapper into kiwix.css.

Copy link
Collaborator

@kelson42 kelson42 left a comment

Choose a reason for hiding this comment

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

LGTM, only a few remarks:

  • 300px is too small. Not sure about the right attitude: removing this or putting something significantly larger. Probably putting 600px is the best compromise.
  • I don't understand why putting the new CSS rules in kiwix.css would be better. Looks good to me like it is.

@veloman-yunkan
Copy link
Collaborator Author

  • I don't understand why putting the new CSS rules in kiwix.css would be better. Looks good to me like it is.

autoComplete.css was imported from another project.

The width of the suggestion box is capped at 600 pixels. The full
text of a suggestion is shown on hover (doesn't work on mobile).
@veloman-yunkan veloman-yunkan force-pushed the fully_visible_suggestions branch from e15d872 to 2eea613 Compare February 3, 2025 12:12
@veloman-yunkan
Copy link
Collaborator Author

  • 300px is too small. Not sure about the right attitude: removing this or putting something significantly larger. Probably putting 600px is the best compromise.

Done

@kelson42 kelson42 merged commit 6b74395 into main Feb 3, 2025
17 of 19 checks passed
@kelson42 kelson42 deleted the fully_visible_suggestions branch February 3, 2025 12:25
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.

Kiwix Server suggestion can not really handle longer titles
2 participants