widgets: Support markdown in widgets. #28079

roanster007 · 2023-12-07T12:36:02Z

Previously, markdown was not supported in widgets like polls and todo. This is fixed by adding an inline only backend processor ZulipInlineMarkdown, and using it to render the poll options or tasks and descriptions of todo before adding them to the database.

Commt 1 (fbb12bc): Add ZulipInlineMarkdown processor to render inline only patterns.
Commit 2 (e4ceba6): Support inline markdown in polls.
Commit 3 (1197b5f): Support inline markdown in todo widgets.

Fixes: #21917

CZO

Screenshots and screen captures:

Self-review checklist

Self-reviewed the changes for clarity and maintainability
(variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

Explains differences from previous plans (e.g., issue description).
Highlights technical choices and bugs encountered.
Calls out remaining decisions and concerns.
Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

Each commit is a coherent idea.
Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

Visual appearance of the changes.
Responsiveness and internationalization.
Strings and tooltips.
End-to-end functionality of buttons, interactions and flows.
Corner cases, error conditions, and easily imagined bugs.

bupd · 2023-12-12T16:17:44Z

@roanster007 Reviewed the changes locally <time and basic markdown seems to be working fine.

I've noticed that the current process of adding time to a poll in Zulip involves typing the time in the compose box and then copying it to the "Add Option" field. While this works well, it would be even more user-friendly to directly implement time rendering for additional options.

I suggest extending the time rendering functionality to the "Add Option" field, allowing users to input time in a similar manner. This enhancement would streamline the process and provide a consistent experience across the poll creation interface.

Implementation Details:

When users type a time in the "Add Option" field, the system should recognize and render it accordingly.
Ensure that the time rendering follows the same format as when entered in the compose box.
Benefits:

Improved user experience by eliminating the need to copy-paste time from the compose box to the "Add Option" field.
Consistency in time rendering across different input fields.
I believe this enhancement would make the poll creation process even more efficient and user-friendly. I'd be happy to contribute to the implementation if needed.

The decision should be made by project contributors to include the above suggestion or not.

bupd

It works.

alya · 2024-01-04T18:58:41Z

Thanks! For polls, links and emoji look good to me, but the time should be rendered like it is in messages. Compare:

alya · 2024-01-04T19:00:16Z

For todo lists:

The elements should work in the description part of the item as well, not just the first part.
These changes break the crossed out formatting for checked items.

alya · 2024-01-04T19:01:15Z

@bupd I agree that we should ideally have a better way to input times (and emoji), but let's make sure we get the rendering part of this working first.

roanster007 · 2024-01-11T18:50:20Z

@alya I have updated the PR and screenshots as per the feedback.

alya · 2024-01-17T23:12:23Z

Thanks! It looks like the space after the : and before the descriptions might have disappeared?

alya · 2024-01-17T23:13:38Z

Other than that, the screenshots look good to me. You can add the "maintainer review" label (via Zulipbot) and ping @N-Shar-ma to take a look once that's been fixed.

roanster007 · 2024-01-20T06:07:45Z

@zulipbot add "maintainer review"

roanster007 · 2024-01-20T06:09:38Z

@N-Shar-ma could you have a look at this..

N-Shar-ma

Reviewed the frontend code and have left some comments.

Someone else who is familiar with the backend markdown processor will be much better suited to review those parts; sorry I've not worked on that part of the codebase so far

N-Shar-ma · 2024-01-28T00:24:11Z

web/src/poll_widget.ts

        const html = render_widgets_poll_widget_results(widget_data);
        $elem.find("ul.poll-widget").html(html);
-
+        render_poll_timestamp($elem);


Directly pass $elem.find("ul.poll-widget") instead to this function. That will make it more generalised, so also reusable by todo widgets, eliminating the near duplicate function in todo_widget.js

Would be even better if you could extract this as a function in rendered_markdown where this code is from, and call that here.

We always want to minimize code duplication as much as possible

I felt using the update_elements of rendered_markdown would be unreasonable since that method would try to look out for lot of other elements too apart form time in $elem like mentions, codehilite,... but those for sure won't be rendered inside the widget by the inline processor.

Moreover, importing rendered_markdown.js method into poll_widget.ts is causing Could not find a declaration file for module './rendered_markdown'. 'rendered_markdown.js' implicitly has an 'any' type

I meant extracting just the relevant code as a new function and calling that, not the pre existing whole update_elements function.

But yes, the non typescript file import into this typescript file still remains an issue 🤔

N-Shar-ma · 2024-01-28T00:41:58Z

web/styles/widgets.css

@@ -166,7 +181,7 @@
    .poll-names {
        color: hsl(0deg 0% 45%);
        padding-left: 4px;
-        padding-top: 28px;
+        padding-top: 2px;


This can be increased to 3 or 4px... It currently looks a little off to me:

Actually you might have to rethink the html structure here... For a multiline option the voters' names take up an entire column of space:

How about now?

Hmm, the 1st line being spaced extra from the rest due to the vote count button looks a little odd... I think we want to keep that button in its own column, but keep the option and voter names inline. Would this be possible?

does this look good?

N-Shar-ma · 2024-01-28T00:50:05Z

web/templates/widgets/todo_widget_tasks.hbs

@@ -8,9 +8,9 @@
            </div>
            <div>
                {{#if completed}}
-                <strike><strong>{{ task }}</strong>{{#if desc }}: {{ desc }}{{/if}}</strike>
+                <strike><strong class="task_complete">{{{ task }}}</strong>{{#if desc }}<span class="task_desc_complete"">: </span><span class="description_complete"">{{{ desc }}}</span>{{/if}}</strike>


There's an extra " after the task_desc_complete class... Also, can you explain why these new classes are needed?

Extra " after description_complete class too

Oh sorry... I missed it. So the reason for these new classes was -- task, : and description are now rendered as separate elements. So without them, sometimes the : is not staying inline, while other times, all three of them go to the next line. So I thought I would enclose them within tags to make them inline separately.

This commit creates an inline only markdown processor, that renders inline patterns.

This commit allows polls to render inline patterns using the inline markdown processor. Fixes zulip#21917.

This commit allows todo to render inline patterns using the inline markdown processor.

nimishmedatwal · 2024-03-28T10:48:07Z

I believe this pr also fixes #21674 #16821 #12947 and #21674.

zulipbot · 2024-04-14T05:34:58Z

Heads up @roanster007, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

zulipbot added size: XL area: widgets Poll and similar widgets feedback wanted priority: high labels Dec 7, 2023

roanster007 force-pushed the iss-12947 branch 2 times, most recently from fd6cc8a to e66720e Compare December 7, 2023 13:13

roanster007 marked this pull request as draft December 9, 2023 20:53

roanster007 marked this pull request as ready for review December 11, 2023 14:35

roanster007 force-pushed the iss-12947 branch 5 times, most recently from 0c7d818 to e7c9978 Compare December 11, 2023 17:03

bupd approved these changes Dec 12, 2023

View reviewed changes

roanster007 force-pushed the iss-12947 branch 3 times, most recently from 3c80967 to d4bae1a Compare January 11, 2024 18:23

This was referenced Jan 15, 2024

Poll time widget #17360

Closed

Add <time> mention markdown parsing to polls #17433

Closed

roanster007 force-pushed the iss-12947 branch from d4bae1a to 0ba0320 Compare January 20, 2024 06:01

zulipbot added the maintainer review PR is ready for review by Zulip maintainers. label Jan 20, 2024

N-Shar-ma suggested changes Jan 28, 2024

View reviewed changes

roanster007 force-pushed the iss-12947 branch from 0ba0320 to 8c1b86e Compare February 2, 2024 15:28

roanster007 requested a review from N-Shar-ma February 2, 2024 15:52

roanster007 force-pushed the iss-12947 branch 2 times, most recently from cf1080f to 1197b5f Compare February 12, 2024 12:57

timabbott added the integration review Added by maintainers when a PR may be ready for integration. label Feb 13, 2024

zulipbot added the has conflicts label Feb 14, 2024

roanster007 force-pushed the iss-12947 branch from 1197b5f to 98539bd Compare February 16, 2024 06:27

zulipbot removed the has conflicts label Feb 16, 2024

alya removed the maintainer review PR is ready for review by Zulip maintainers. label Feb 16, 2024

roanster007 added 3 commits March 10, 2024 23:25

markdown: Create an inline markdown processor.

055a2e7

This commit creates an inline only markdown processor, that renders inline patterns.

polls: Render markdown in polls.

b382f60

This commit allows polls to render inline patterns using the inline markdown processor. Fixes zulip#21917.

todo: Render markdown in todo widgets.

52b63db

This commit allows todo to render inline patterns using the inline markdown processor.

roanster007 force-pushed the iss-12947 branch from 98539bd to 52b63db Compare March 10, 2024 18:34

zulipbot added the has conflicts label Apr 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

widgets: Support markdown in widgets. #28079

widgets: Support markdown in widgets. #28079

roanster007 commented Dec 7, 2023 •

edited

Loading

bupd commented Dec 12, 2023

bupd left a comment

alya commented Jan 4, 2024

alya commented Jan 4, 2024

alya commented Jan 4, 2024

roanster007 commented Jan 11, 2024

alya commented Jan 17, 2024

alya commented Jan 17, 2024

roanster007 commented Jan 20, 2024

roanster007 commented Jan 20, 2024

N-Shar-ma left a comment

N-Shar-ma Jan 28, 2024

N-Shar-ma Jan 28, 2024

roanster007 Feb 2, 2024 •

edited

Loading

N-Shar-ma Feb 5, 2024

N-Shar-ma Jan 28, 2024

N-Shar-ma Jan 28, 2024

roanster007 Feb 2, 2024

N-Shar-ma Feb 5, 2024

roanster007 Feb 11, 2024 •

edited

Loading

N-Shar-ma Jan 28, 2024

N-Shar-ma Jan 28, 2024

roanster007 Feb 2, 2024

nimishmedatwal commented Mar 28, 2024

zulipbot commented Apr 14, 2024

widgets: Support markdown in widgets. #28079

Are you sure you want to change the base?

widgets: Support markdown in widgets. #28079

Conversation

roanster007 commented Dec 7, 2023 • edited Loading

bupd commented Dec 12, 2023

bupd left a comment

Choose a reason for hiding this comment

alya commented Jan 4, 2024

alya commented Jan 4, 2024

alya commented Jan 4, 2024

roanster007 commented Jan 11, 2024

alya commented Jan 17, 2024

alya commented Jan 17, 2024

roanster007 commented Jan 20, 2024

roanster007 commented Jan 20, 2024

N-Shar-ma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roanster007 Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roanster007 Feb 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nimishmedatwal commented Mar 28, 2024

zulipbot commented Apr 14, 2024

roanster007 commented Dec 7, 2023 •

edited

Loading

roanster007 Feb 2, 2024 •

edited

Loading

roanster007 Feb 11, 2024 •

edited

Loading