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

Handle <ol start=…> #59

Open
gnprice opened this issue Apr 10, 2023 · 7 comments · May be fixed by #1329
Open

Handle <ol start=…> #59

gnprice opened this issue Apr 10, 2023 · 7 comments · May be fixed by #1329
Assignees
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents beta feedback Things beta users have specifically asked for help wanted
Milestone

Comments

@gnprice
Copy link
Member

gnprice commented Apr 10, 2023

An <ol> element in Zulip HTML can have the start attribute, like so:

<ol start="9999">
<li>Fjfjdj</li>
<li>Fjdjdj</li>
</ol>

On Zulip web, this renders like so:
image

(The fact that the numbers are cut off at the left is a bug that's getting fixed: zulip/zulip#25063.)

Currently we ignore start and always start the numbering at 1. We should instead respect start when we see it.

This means:

  • When we parse the HTML, ListNode should get a bit more information to record start. Possibly make it abstract and give it subclasses OrderedListNode and UnorderedListNode, since start won't make sense on the latter.
  • Then ListNodeWidget should act on that information.
@gnprice gnprice added the a-content Parsing and rendering Zulip HTML content, notably message contents label May 27, 2023
@gnprice
Copy link
Member Author

gnprice commented Jun 2, 2023

The Zulip Markdown message source that produces the example HTML above is:

9999. Fjfjdj
10000. Fjdjdj

@gnprice
Copy link
Member Author

gnprice commented Jun 3, 2023

As part of this issue, the changes to HTML parsing should get tests in test/model/content_test.dart. This will probably involve adding another case to ContentNodeChecks.equalsNode (in content_checks.dart) for ListNode, similar to the existing case for ParagraphNode.

Ideally the changes to ListNodeWidget should also get tests, in test/widgets/content_test.dart. But we don't yet really have an example of that kind of test, so it's OK to leave that out for this issue. We have fewer examples of these, and I think that's a lower-priority test for this code (because that part seems less likely to later get broken by accident), so if that test feels hard to write then it's OK to leave it out for this issue.

(Edited 2023-07-10 with updates.)

@chplALEX

This comment was marked as outdated.

@gnprice

This comment was marked as outdated.

@gnprice gnprice added this to the Launch milestone Jun 15, 2023
@gnprice gnprice modified the milestones: Launch, B2: Summer 2024 May 9, 2024
@chrisbobbe chrisbobbe added the beta feedback Things beta users have specifically asked for label Dec 27, 2024
@chrisbobbe
Copy link
Collaborator

@lakshya1goel
Copy link
Contributor

Hi, I have started working on this issue. Sharing the screenshots shortly.
Thanks!

lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 5, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 6, 2025
@lakshya1goel
Copy link
Contributor

Hi, I have raised PR #1329 for this issue, PTAL. Thanks!

lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 12, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 13, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 14, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 14, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 21, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents beta feedback Things beta users have specifically asked for help wanted
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

4 participants