Skip to content

Refactor content widgets toward touchable links #209

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

Merged
merged 5 commits into from
Jun 30, 2023

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Jun 29, 2023

This is another segment of my draft branch #204, toward #71.

I think these refactors stand on their own, but in particular they will be helpful for (a) creating a stateful widget to hold gesture recognizers for links in a given BlockInlineContainerNode, and (b) threading those recognizers down through building the inline spans for that node's subtree.

gnprice added 5 commits June 30, 2023 12:14
Making lists non-growable is a small optimization that's
particularly helpful when creating a large number of short lists.

That describes what we do here, mapping inline content nodes to
corresponding inline spans: there will be tons of these nodes in a
message list; tons of them will be in lists of one element (like
any paragraph of plain text, or any link or strong/bold or
emphasis/italic span with no further markup inside it); and
probably the bulk of the rest will be in lists of two or three.

So this is a spot where the optimization may well have a material
performance impact.  In any case it's essentially free; since we
didn't want to add or remove elements anyway, the only cost is the
extra few tokens in the source code.
This was basically the only appropriate way to use the return value
of _buildInlineList, so it might as well get pushed inside the
function itself.
@chrisbobbe chrisbobbe force-pushed the pr-content-widgets branch from 55727dd to a6dc643 Compare June 30, 2023 19:18
@chrisbobbe chrisbobbe merged commit a6dc643 into zulip:main Jun 30, 2023
@chrisbobbe
Copy link
Collaborator

Thanks! LGTM; merged.

@gnprice gnprice deleted the pr-content-widgets branch June 30, 2023 19:19
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