-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
@mention list should be scrollable #20620
Comments
@zulipbot claim |
Hello @thecalendar! Thanks for your interest in Zulip! You have attempted to claim an issue without the labels "help wanted", "good first issue". Since you're a new contributor, you can only claim and submit pull requests for issues with the help wanted or good first issue labels. If this is your first time here, we recommend reading our guide for new contributors before getting started. |
@asah I am a newbie here and wished to contribute. If you think this issue is a good first issue, can you add the necessary label so that I can work on it? Thank you. |
I'm also a newbie! But yes this seems like a good starter project for someone sufficiently should in JavaScript. Be careful to test on different browsers and (the hard part) setup puppeteer tests. I'm happy to help! My contact info is in my GitHub profile. |
I've wanted to have such a scrolling feature as well, but I don't think this is ready for a new contributor to work on unless they're prepare to do a lot of research themselves, in that there's some things going on here. It may be somewhat complex to do infinite scrolling perfectly, because of performance optimizations in how we implement the sort/filter/matching process for organizations with 20K+ users. But I think it'd still be an improvement if we can retune the performance optimizations to, say, be guaranteed to have the first 50 results be correct, and let you scroll through up to 50 items? Or maybe there's a more clever hack available here.
@showell do you remember this code path enough to say whether we could just change the cutoff to 50 there, add a scrollbar, and then just need to work on the UI side of this idea? |
Y I'm familiar with the issues in scaling, backend, databases and search
engines. IMHO start with a demo and also check single user scaling, and
then worry about multiple users, DDOS/overload, caching, etc. Also rollout
slowly to beta testers vs whole orgs. Testing search quality gets tricky...
I have concerns about per-user and per-use-case search quality issues. In
large orgs with many teams & projects, this gets quite tricky - Gmail did a
nice job recently but even they've got big quality gaps.
(I've seen this for decades, and have some ideas to try... Zulip is
wonderfully hackable, so I'm optimistic!)
More coming...
…On Tue, Jan 4, 2022, 7:09 PM Tim Abbott ***@***.***> wrote:
I've wanted to have such a scrolling feature as well, but I don't think
this is ready for a new contributor to work on unless they're prepare to do
a lot of research themselves, in that there's some things going on here.
It may be somewhat complex to do infinite scrolling perfectly, because of
performance optimizations in how we implement the sort/filter/matching
process for organizations with 20K+ users. But I think it'd still be an
improvement if we can retune the performance optimizations to, say, be
guaranteed to have the first 50 results be correct, and let you scroll
through up to 50 items? Or maybe there's a more clever hack available here.
const filtered_groups = groups.filter((item) => query_matches_name_description(query, item));
/*
Let's say you're on a big realm and type
"st" in a typeahead. Maybe there are like
30 people named Steve/Stephanie/etc. We don't
want those search results to squeeze out
groups like "staff", and we also want to
prefer Steve Yang over Stan Adams if the
former has sent messages recently, despite
the latter being first alphabetically.
Also, from a performance standpoint, we can
save some expensive work if we get enough
matches from the more selective group of
people.
Note that we don't actually guarantee that we
won't squeeze out groups here, but we make it
less likely by removing some users from
consideration. (The sorting step will favor
persons who match on prefix to groups who
match on prefix.)
*/
const cutoff_length = max_num_items;
const filtered_message_persons = filter_persons(people.get_active_message_people());
let filtered_persons;
if (filtered_message_persons.length >= cutoff_length) {
filtered_persons = filtered_message_persons;
} else {
filtered_persons = filter_persons(people.get_realm_users());
}
return typeahead_helper.sort_recipients({
users: filtered_persons,
query,
current_stream: opts.stream,
current_topic: opts.topic,
groups: filtered_groups,
max_num_items,
});
@showell <https://github.com/showell> do you remember this code path
enough to say whether we could just change the cutoff to 50 there, add a
scrollbar, and then just need to work on the UI side of this idea?
—
Reply to this email directly, view it on GitHub
<#20620 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABCFYTCTDM2VJSWN7JGZKDUUOD5HANCNFSM5KTTDLJQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Wow, thx @timabbott for the details above... Strictly for prototyping, I dug into the code, read-up on bootstrap-typeahead and found a potential quick solution for smaller installations. I'm deploying it to my users (currently ~80) to get feedback over the next few days. the thinking: auto-detect smaller installations (method tbd) where scaling isn't an issue and turn something-like-this on by default, then consider a longer/bigger project to decide how to handle larger installations (which presumably provide the funding for zulip...) diff --git a/static/js/composebox_typeahead.js b/static/js/composebox_typeahead.js // This is what we use for PM/compose typeaheads. export let emoji_collection = []; diff --git a/static/styles/compose.css b/static/styles/compose.css .dropdown-menu {
thanks to https://stackoverflow.com/a/34660117/430938 Obviously, need to replace "150px" with something responsive... sorry, showing my newbie stripes! p.s. in case I don't say it enough, it's awesome how hackable zulip is: from a cold start, this took me an hour to right-click in chrome devmode to find the html/css, grep the code, find bootstrap-typeahead, find max_num_items, run a few google searches for others who setup scrolling with bootstrap-typeahead, figure out how to generate 100 users (I ended up choosing manage.py + bash for-loop), test, commit and deploy. In 40 years of coding, maybe Borland Pascal was like this. Kudos!!! |
I am 99% sure that the constraint of showing only 5 or so results was a UI-driven policy decision, and it doesn't play any significant role in helping us search large user populations in the ~20k range. I think showing 50 results would probably be reasonable once we had the scrolling UI worked out. I think a true infinite scroll would be tougher performance-wise. |
y - please try the patch - it keeps the same space on the screen while
adding scroll as necessary. Arbitrarily, I extended the limit to 1,000
results, but you can set it lower if you like.
…On Thu, Jan 6, 2022 at 1:16 PM Steve Howell ***@***.***> wrote:
I am 99% sure that the constraint of showing only 5 or so results was a
UI-driven policy decision, and it doesn't play any significant role in
helping us search large user populations in the ~20k range. I think showing
50 results would probably be reasonable once we had the scrolling UI worked
out. I think a true infinite scroll would be tougher performance-wise.
—
Reply to this email directly, view it on GitHub
<#20620 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABCFYSX4N3VTVQCCW2ZJBDUUXMADANCNFSM5KTTDLJQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@N-Shar-ma do you want to try putting together a PR doing this? |
@timabbott I'm yet to read everything properly and understand the details of what's needed, but yes I do want to try this out! |
Using simplebar. Fixes: zulip#20620.
Using simplebar. Fixes: zulip#20620.
To increase the number of options available for the user to pick from, we increase the limit of options shown to 50 from 8, and make the menu scrollable, while retaining its original dimensions. Fixes: zulip#20620.
To increase the number of options available for the user to pick from, we increase the limit of options shown to 50 from 8, and make the menu scrollable, while retaining its original dimensions. Fixes: zulip#20620.
To increase the number of options available for the user to pick from, we increase the limit of options shown to 50 from 8, and make the menu scrollable, while retaining its original dimensions. Fixes: zulip#20620.
To increase the number of options available for the user to pick from, we increase the limit of options shown to 50 from 8, and make the menu scrollable, while retaining its original dimensions. Fixes: zulip#20620.
To increase the number of options available for the user to pick from, we increase the limit of options shown to 50 from 8, and make the menu scrollable, while retaining its original dimensions. Fixes: zulip#20620.
To increase the number of options available for the user to pick from, we increase the limit of options shown to 50 from 8, and make the menu scrollable, while retaining its original dimensions. Fixes: zulip#20620.
To increase the number of options available for the user to pick from, we increase the limit of options shown to 50 from 8, and make the menu scrollable, while retaining its original dimensions. Fixes: zulip#20620.
For installations with more than a half dozen users :-)
Note: this requires non-trivial JavaScript hacking.
Edit by @alya (2023-11):
Currently blocked on a a rewrite of the typeahead library's HTML interface logic on top of Tippy. CZO discussion
The text was updated successfully, but these errors were encountered: