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

[WIP] Implement has: client side filter #15363

Closed
wants to merge 2 commits into from

Conversation

thedeveloperr
Copy link
Collaborator

#6186

Testing Plan:

GIFs or Screenshots:

} else if (operand === 'attachment') {
return message.has_attachment;
}
return false; // is:whatever returns true
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I don't think we have to send the data from the server; fundamentally, we can just iterate through the link tags in the HTML via jQuery to do a completely client-side check for each of these.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok I will try that approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@timabbott I am able to detect images, link
but how to distinguish between normal links and attachments like pdf, mp4, etc. ?
Can i rely on checking if /user_uploads/ exist in the href of the anchor tag ?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Yes, attachment has a very specific meaning: a link to a /user_uploads URL.

Prior to this commit has:link, has:attachment, has:image
filter couldn't be applied locally and deferred filtering to
web server. This commits make sure client filters all messages
it can instead of completely deferring to the server and hence
improve speed.

A tradeoff is also made to turn off local echo for has: narrows
as messages with link sent to has:link narrow were locally echoing
to another narrow and not appearing in the active has:link narrow.

Fixes: zulip#6186.
@thedeveloperr
Copy link
Collaborator Author

Closing in favour of #15492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants