Skip to content

move autocomplete list rendering to client side #9832

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

johndoh
Copy link
Contributor

@johndoh johndoh commented Mar 31, 2025

Moving the rendering of the autocomplete address list used in compose to the client side allows skins to handle it and offer different layouts for different sized screens. Before I spend too much time on this I wanted to submit this draft to see if the idea was worth pursuing.

There are some hurdles. There is the contact_search_name config var which has to be respected. The server side needs to know about it so it can return the correct fields in the request but rendering those is a client side task because of screen size. Also contact_search_name does not support HTML and I think that's correct, it could be abused. May be having a configurable display for normal screens and a fixed one for small screens is enough.

Example of how the new small screen layout looks:
compose

The proposed rcube_webmail.ksearch_results_display() and rcube_webmail.ksearch_results_highlight() JS functions can be overridden by skins/plugins if required but the stock version maintain the current functionality plus a new layout for smaller screens.

I think ultimately all the text rendering code should be removed from rcube_addressbook::compose_search_name() and instead only the fields returned with the rendering moved into app.js.

This might help with #9329 and #9219.

@alecpl
Copy link
Member

alecpl commented Mar 31, 2025

We should consider removing contact_search_name option. Maybe we can replace it with autocomplete_extra_field or something. This way the result would contain at most 3 items: "email", "display" (contact name), "extra" (e.g. department name). The extra text could be displayed not inline, but maybe aligned to the right, and only if there's room.

We can consider displaying "two line" records on a big screen too. Gmail does that.

@johndoh
Copy link
Contributor Author

johndoh commented Mar 31, 2025

I like the idea of scrapping contact_search_name. The autocomplete_extra_field, I think it would make sense if that could only be 0 or 1 fields and then skins have a fixed layout to support. If devs want more fields or more customisation then they could do it via the plugin API because its not only about the fields but also the layout. Does that sound acceptable?

Do we really need autocomplete_extra_field even, could addressbook_name_listing be used/improved so that if you wanted say firstname lastname (department) its shown the same in the autocomplete and the contact list? Then in autocomplete the email address would be on the second line and that's the only difference with the contacts list. It could use similar logic as that currently used for displaying contact_search_name so no existing configurability would be lost.

Off the top of my head I'm thinking a plugin hook in rcube_addressbook::compose_search_name() for the returned fields/values and then use the proposed (or similar) new JS functions to construct the entry.

@alecpl
Copy link
Member

alecpl commented Apr 1, 2025

Makes sense.

@johndoh
Copy link
Contributor Author

johndoh commented Apr 5, 2025

I have replaced contact_search_name with contactlist_name_template which applies in the contact list and the autocomplete, anywhere addressbook_name_listing is used. I did not change addressbook_name_listing because of its use in the settings screen.

eef8919 I modified the ACL plugin to use the new autocomplete but I do not have a working LDAP setup so I cannot test this change, please could someone who uses this plugin confirm the autocomplete still works?

@johndoh johndoh marked this pull request as ready for review April 5, 2025 09:28
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.

2 participants