Add dataTable plugin #2446

Tim-Siu · 2024-03-09T04:20:51Z

What is the purpose of this pull request?

Overview of changes:

Contribute to #896

Added a dataTable plugin. When we put a table into <m-table sortable> tag, it will become a Sortable Table. When we put a table into <m-table searable> tag, it will become a Searchable Table. Both functions can be used together.

DataTable is used.

Sample usage:

<m-table sortable searchable>
| Name | Age | Country |
|------|-----|---------|
| John | 28  | USA     |
| Emily| 32  | Canada  |
| Michael | 41 | UK    |
| Sophia | 25 | Germany|
| David | 37  | Australia |
</m-table>

Result:

Screen.Recording.2024-03-14.at.21.43.26.mov

Anything you'd like to highlight/discuss:
This has the potential of breaking the rendering pipeline. I am still conducting tests.

Testing instructions:
See overview.

Proposed commit message: (wrap lines at 72 characters)

Add dataTable plugin

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

Major (when you make incompatible API changes)
Minor (when you add functionality in a backward compatible manner)
Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

To be included in the release note for any feature that is made obsolete/breaking

Previously tables do not support searching and sorting.
Authors can now enable these features by adding dataTable as a site plugin and add the respective syntax around the table.

Give a brief explanation note about:

what was the old feature that was made obsolete

any replacement feature (if any), and

how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

codecov · 2024-03-09T04:23:17Z

Codecov Report

Attention: Patch coverage is 96.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 51.05%. Comparing base (cb59668) to head (e254c05).

Files	Patch %	Lines
packages/core/src/plugins/dataTable.ts	96.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2446      +/-   ##
==========================================
+ Coverage   50.80%   51.05%   +0.25%     
==========================================
  Files         125      126       +1     
  Lines        5437     5467      +30     
  Branches     1170     1174       +4     
==========================================
+ Hits         2762     2791      +29     
- Misses       2380     2381       +1     
  Partials      295      295

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yucheng11122017

Hi @Tim-Siu thanks for the work on this!

Before you continue, I think one is that this will always sort by text.. which is not necessarily what is needed. For example, the example that you gave perhaps you want to sort by numbers instead? But currently, if you add 28 vs 201, 201 will come before 28.

Also, could you make sure that this will work for external imports as well? I think it doesn't work for now.

yiwen101 · 2024-03-09T18:46:30Z

Hi @Tim-Siu thank you for the amazing work;
After settling other issues and reviews, could you also help update the user doc regrading this?

damithc · 2024-03-10T05:05:21Z

Nice effort @Tim-Siu
Some things to think about:

sortable sounds like an attribute, so something like <m-table sortable> seems more intuitive? Allows to add more attributes later too.
For number columns, sorting should be by value (i.e., 2 is smaller than 15), not lexicographic. Is it handled automatically? Or is there a way for the user to specify? e.g., <m-table sortable sorting="text, number, text">

Solve the problem that the original `markdown-it` is modified

Merge master

itsyme · 2024-03-12T02:41:37Z

Great addition to tables @Tim-Siu!

I just have two small nits from a UI perspective.

Perhaps you may want to explore other options for the icon for the unordered state (currently arrow up and down) to something that depicts its function more clearly. In my opinion the current icon is resembles cursor: resize giving me an impression that I can resize/move the column.
I felt that the current design may be a little too distracting. Perhaps something like decreasing the opacity and upping it on hover could declutter the column headers a little. You can take some inspiration from MUI Tables

Great work so far!

kaixin-hc · 2024-03-12T04:37:00Z

looks p good to me! - but wondering 2 things

possible to add tests?
if we're allowing sorting for text and numbers, is it possible / useful to also support sorting by date? if its simple we can do it here if its troublesome perhaps another PR

Tim-Siu · 2024-03-12T04:44:20Z

Great addition to tables @Tim-Siu!

I just have two small nits from a UI perspective.

Perhaps you may want to explore other options for the icon for the unordered state (currently arrow up and down) to something that depicts its function more clearly. In my opinion the current icon is resembles cursor: resize giving me an impression that I can resize/move the column.

I felt that the current design may be a little too distracting. Perhaps something like decreasing the opacity and upping it on hover could declutter the column headers a little. You can take some inspiration from MUI Tables

Great work so far!

Sure! I will explore different UI. I think there are some existing implementations on the internet for bootstrap tables and I will try to refer to them.

Add support for searching Improve TS code style

Tim-Siu · 2024-03-13T15:56:16Z

Hi @damithc ,

I updated the plugin and it is now able to support both sorting and filtering. This is all, however, powered by DataTables, which is a Javascript table library that is loaded from CDN directly. What do you think of the functioinality now? There are additional features like paging and scrolling from DataTable. Are these features needed?

Thanks for your advice.

damithc · 2024-03-13T16:31:32Z

I updated the plugin and it is now able to support both sorting and filtering. This is all, however, powered by DataTables, which is a Javascript table library that is loaded from CDN directly. What do you think of the functioinality now? There are additional features like paging and scrolling from DataTable. Are these features needed?

@Tim-Siu Good find.
I'm no objections to using a third-party plugin, if the license is permissible. Another issue is the reliability of the CDN.
What's the syntax for enabling filtering and sorting?
What are the other features you think that might be relevant to our context?

damithc · 2024-03-13T16:56:48Z

Other concerns to check on:

How mature the plugin is
Is it currently active and likely to stay active for many years
The likelihood that the free version will be crippled in some ways in future, to push people to buy the commercial version

Merge master

Tim-Siu · 2024-03-14T05:45:13Z

Other concerns to check on:

How mature the plugin is

Is it currently active and likely to stay active for many years

The likelihood that the free version will be crippled in some ways in future, to push people to buy the commercial version

Hi @damithc ,

Thank you for your input!

Regarding your concern:

The syntax for the plugin is that <m-table sortable searchable> to enable both features. We may also include only one feature.
The plugin appears to be quite popular and active. It is open source under MIT license. I have included the minimised js and css in our repo. It should be working fine as long as we are still using Bootstrap5. I guess we can still stick with these js and css files even if they commercialize it?
Some other features include paging. But I fell like it is a bit distracting as it wil add page info and agination control. It may be useful if we intend to have a really long table. But I think we already have panel, so it is not a necessary feature.

I do realise that the plugin is not working inside a panel or modal. It seems to suffer the same problem as the mermaid plugin #2052 . I am still investigating this issue.

damithc · 2024-03-14T12:05:42Z

I do realise that the plugin is not working inside a panel or modal. It seems to suffer the same problem as the mermaid plugin #2052 . I am still investigating this issue.

@Tim-Siu I see. That's a bummer, but if we figure out a way to fix it, we might be able to add both this and Mermaid to MarkBind, which would be REALLY great. Feel free to join forces with other team members (including mentors) to explore this, as this is likely hard to solve.

yucheng11122017 · 2024-03-14T13:38:02Z

Hi @Tim-Siu, can I check how to use this feature? I have added dataTable to the plugins and then used

<m-table sortable searchable>
| Name | Age | Country |
|------|-----|---------|
| John | 28  | USA     |
| Emily| 32  | Canada  |
| Michael | 41 | UK    |
| Sophia | 25 | Germany|
| David | 37  | Australia |
</m-table>

But it doesn't seem to work.

Please add some documentation so that we can test it easily.

Tim-Siu · 2024-03-14T13:46:37Z

Hi @Tim-Siu, can I check how to use this feature? I have added dataTable to the plugins and then used
<m-table sortable searchable>
| Name | Age | Country |
|------|-----|---------|
| John | 28  | USA     |
| Emily| 32  | Canada  |
| Michael | 41 | UK    |
| Sophia | 25 | Germany|
| David | 37  | Australia |
</m-table>
But it doesn't seem to work.

Please add some documentation so that we can test it easily.

Hi @yucheng11122017 ,

That is the correct usage.

It does work on my side. Can you paste a screenshot of the output on your side?

Screen.Recording.2024-03-14.at.21.43.26.mov

--------- Co-authored-by: Chan Yu Cheng <[email protected]>

yucheng11122017

Some nits and clean up!

yucheng11122017 · 2024-04-16T15:31:42Z

docs/userGuide/plugins/dataTable.md

+
+To create a table with DataTable features, use one of the following syntaxes:
+
+1. Sortable Table:


As I mean lines 19 to 52 is almost the same as 54 to 100 so its repetitive. We can delete 19 to 52 I think!

yucheng11122017 · 2024-04-16T15:33:22Z

packages/core/src/plugins/dataTable.ts

+
+        let tableClass: string = '';
+        if (isSortable && isSearchable) {
+          tableClass += ' sortable-searchable-table';


Suggested change

tableClass += ' sortable-searchable-table';

tableClass = ' sortable-searchable-table';

yucheng11122017 · 2024-04-16T15:33:28Z

packages/core/src/plugins/dataTable.ts

+        if (isSortable && isSearchable) {
+          tableClass += ' sortable-searchable-table';
+        } else if (isSortable) {
+          tableClass += ' sortable-table';


Suggested change

tableClass += ' sortable-table';

tableClass = ' sortable-table';

yucheng11122017 · 2024-04-16T15:33:34Z

packages/core/src/plugins/dataTable.ts

+        } else if (isSortable) {
+          tableClass += ' sortable-table';
+        } else if (isSearchable) {
+          tableClass += ' searchable-table';


Suggested change

tableClass += ' searchable-table';

tableClass = ' searchable-table';

yucheng11122017 · 2024-04-16T15:38:18Z

packages/cli/test/functional/test_site_table_plugin/site.json

+    {
+      "glob": ["**/index.md", "**/*.md"]
+    },
+    {


Please delete these two pages - they aren't needed for testing!

* Change seamless panels * Improve look of seamless panels * Add tests * Update userguide example --------- Co-authored-by: Chan Yu Cheng <[email protected]> Co-authored-by: Hannah <[email protected]>

itsyme

Just two small nits to make the documentation and code slightly more readable! Almost there!

docs/userGuide/plugins/dataTable.md

itsyme · 2024-04-17T08:50:19Z

packages/core/src/plugins/dataTable.ts

+    $('d-table').each((index: number, node: cheerio.Element) => {
+      const $node = $(node);
+      const html = $node.html();
+      if (html != null) {


My opinion is it would be cleaner to use a guard clause here. i.e. check for html == null instead of html != null and continue/return. This allows the main logic of the code to not be wrapped in the if block, making it cleaner/more readable.

Here's a useful article on guard clauses: https://deviq.com/design-patterns/guard-clause

yucheng11122017

Some comments

docs/userGuide/plugins/dataTable.md

yucheng11122017 · 2024-04-17T14:39:26Z

packages/cli/test/functional/test_site_table_plugin/site.json

+    {
+      "glob": ["**/index.md", "**/*.md"]
+    },
+    {


Hi @Tim-Siu I meant that by putting these two pages in the site.json, a user can basically access these pages by going to
{{baseUrl}}/others/includeInsideTable.html
which isn't necessary in this case. So please delete it.

You can read more about this in the UG

yucheng11122017 · 2024-04-19T06:24:11Z

packages/core/test/unit/plugins/default/dataTable.test.ts

+
+test('postRender should add appropriate classes and attributes to m-table elements', () => {
+  const content = `
+    <m-table sortable>


Isnt this supposed to be d-table now?

Why does the test case still pass when the tag is m-table?

Why does the test case still pass when the tag is m-table?

Thank you for pointing it out. It turns out it is necessary to verify the length of the output. Currently, if I use , the length of the output after the selector is 0, leading to a vacuously true situation.

Fix the problem that there is a additional space in table attribute

yucheng11122017

Some comments

yucheng11122017 · 2024-04-19T07:28:58Z

packages/core/test/unit/plugins/default/dataTable.test.ts

+  ];
+
+  const renderedContent = dataTable.postRender({}, {}, content);
+  console.log(renderedContent);


Do we need this console.log?

Sorry I forgot to clean this up...

yucheng11122017 · 2024-04-19T07:29:09Z

packages/core/test/unit/plugins/default/dataTable.test.ts

+
+test('postRender should add appropriate classes and attributes to m-table elements', () => {
+  const content = `
+<d-table sortable>


Why is the formatting here like this?

It seems like that if there are tabs indentation, it will be rendered as code...I haven't figured out why.

Why is the formatting here like this?

I think this is the bahaviours for all the Vue components. (The behaviours that if we put indentations, the Vue comoponents won't be rendered correctly.

itsyme

LGTM! Ready to be merged after resolving the latest comments from @yucheng11122017!

yucheng11122017

LGTM! Thanks for the work

LamJiuFong · 2024-04-21T14:02:57Z

LGTM! Thanks for the work @Tim-Siu

damithc · 2024-05-03T11:15:14Z

Feature in action: https://nus-cs2103-ay2324s2.github.io/website/schedule/week4/project.html#2-review-some-peer-prs-fri-feb-9th-1600-counted-for-participation

Add SortableTable plugin

ca9703f

yucheng11122017 reviewed Mar 9, 2024

View reviewed changes

Tim-Siu changed the title ~~Add SortableTable plugin~~ [WIP] Add SortableTable plugin Mar 9, 2024

Tim-Siu marked this pull request as draft March 9, 2024 23:41

Tim-Siu added 4 commits March 12, 2024 00:27

Solve include not working

888ce62

Solve the problem that the original `markdown-it` is modified

Solve undesired behavior when sorting numbers

7b1c13b

Update tag to <m-table sortable>

bce158d

Merge branch 'master' into table

40519ac

Merge master

Tim-Siu marked this pull request as ready for review March 11, 2024 16:47

Tim-Siu changed the title ~~[WIP] Add SortableTable plugin~~ Add SortableTable plugin Mar 11, 2024

Switch to use dataTable

ddb6590

Add support for searching Improve TS code style

Tim-Siu changed the title ~~Add SortableTable plugin~~ Add dataTable plugin Mar 13, 2024

Tim-Siu added 3 commits March 14, 2024 12:59

Include css and js in the project

a398c90

Merge branch 'master' into table

a65a158

Merge master

Include datatables js in the project

113f758

Add functional test

d9806d4

Update functional test

aacb32c

yiwen101 and others added 6 commits April 13, 2024 00:33

Fix off-positioned close button in imported modal (MarkBind#2487)

75f0e77

--------- Co-authored-by: Chan Yu Cheng <[email protected]>

Clean up functional test

ba27fb6

Further clean up functional test

8d90deb

Merge branch 'master' into table

f011514

Update documentation

2c6846e

Merge branch 'master' into table

224e29a

yucheng11122017 reviewed Apr 16, 2024

View reviewed changes

jingting1412 and others added 3 commits April 17, 2024 15:02

Enable seamless panels to inherit any parent's colours (MarkBind#2501)

8253f82

* Change seamless panels * Improve look of seamless panels * Add tests * Update userguide example --------- Co-authored-by: Chan Yu Cheng <[email protected]> Co-authored-by: Hannah <[email protected]>

Merge branch 'master' into table

122e9f2

Clean up code and documentation

cf6e91b

itsyme reviewed Apr 17, 2024

View reviewed changes

yucheng11122017 reviewed Apr 17, 2024

View reviewed changes

itsyme force-pushed the master branch from bb8d02f to cb59668 Compare April 19, 2024 02:09

Tim-Siu and others added 3 commits April 19, 2024 12:32

Merge branch 'master' into table

122fbd8

Remove unnecessary pages in site.json

5544ed4

Switch to guard clause

a3180ac

yucheng11122017 reviewed Apr 19, 2024

View reviewed changes

Fix bugs in tests

989836d

Fix the problem that there is a additional space in table attribute

yucheng11122017 reviewed Apr 19, 2024

View reviewed changes

itsyme approved these changes Apr 19, 2024

View reviewed changes

Clean up code

e254c05

yucheng11122017 approved these changes Apr 21, 2024

View reviewed changes

LamJiuFong added the r.Major Version resolver: increment by 1.0.0 label Apr 21, 2024

LamJiuFong merged commit c9f72c4 into MarkBind:master Apr 21, 2024

Tim-Siu mentioned this pull request Apr 24, 2024

Making Plugins (mermaid, datatables) Default #2529

Open

kaixin-hc added r.Minor Version resolver: increment by 0.1.0 and removed r.Major Version resolver: increment by 1.0.0 labels May 12, 2024

Tim-Siu mentioned this pull request Feb 10, 2025

Steps Needed for Vue Migration (Vue 2 -> Vue 3) #2597

Closed

15 tasks


		To create a table with DataTable features, use one of the following syntaxes:

		1. Sortable Table:

	tableClass += ' sortable-searchable-table';
	tableClass = ' sortable-searchable-table';

	tableClass += ' sortable-table';
	tableClass = ' sortable-table';

	tableClass += ' searchable-table';
	tableClass = ' searchable-table';

Add dataTable plugin #2446

Add dataTable plugin #2446

Uh oh!

Conversation

Tim-Siu commented Mar 9, 2024 • edited by kaixin-hc Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yucheng11122017 left a comment

Choose a reason for hiding this comment

Uh oh!

yiwen101 commented Mar 9, 2024

Uh oh!

damithc commented Mar 10, 2024

Uh oh!

itsyme commented Mar 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaixin-hc commented Mar 12, 2024

Uh oh!

Tim-Siu commented Mar 12, 2024

Uh oh!

Tim-Siu commented Mar 13, 2024

Uh oh!

damithc commented Mar 13, 2024

Uh oh!

damithc commented Mar 13, 2024

Uh oh!

Tim-Siu commented Mar 14, 2024

Uh oh!

damithc commented Mar 14, 2024

Uh oh!

yucheng11122017 commented Mar 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tim-Siu commented Mar 14, 2024

Uh oh!

yucheng11122017 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsyme left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

itsyme Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yucheng11122017 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yucheng11122017 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tim-Siu commented Mar 9, 2024 •

edited by kaixin-hc

Loading

codecov bot commented Mar 9, 2024 •

edited

Loading

itsyme commented Mar 12, 2024 •

edited

Loading

yucheng11122017 commented Mar 14, 2024 •

edited

Loading

itsyme Apr 17, 2024 •

edited

Loading

Tim-Siu Apr 20, 2024 •

edited

Loading