added data tables support #273
Conversation
There was a problem hiding this comment.
UI renders, but buttons and search are not functional.
https://pr-preview.s3.amazonaws.com/jricher/did-spec-registries/pull/273.html#did-methods
|
Buttons and search work on my machine, don't know why it isn't working on the Amazon preview. I don't see any console errors to indicate what broke.
|
|
@jricher hmm, if it works for you locally it might only be broken in preview... I am ok merging it to "see if it works" after. |
There was a problem hiding this comment.
The rendering looks workable. I concur with the need to merge and test, in order to know whether it's only broken in the preview or by something Respec is doing. (@iherman, who wasn't on the call where we talked about this, may know of another recommended library or other method to achieve the arbitrary table re-sorting and/or cursor windowing.)
|
| <link rel="stylesheet" type="text/css" href="https://cdn.datatables.net/1.10.24/css/jquery.dataTables.css"> | ||
|
|
||
| <script type="text/javascript" charset="utf8" src="https://cdn.datatables.net/1.10.24/js/jquery.dataTables.js"></script> | ||
|
|
There was a problem hiding this comment.
Alas! I do not think this is acceptable. A document in the W3C's /TR place cannot be dependent on an external library like jQuery (are we sure that this will still work in a few years?).
Also... I expect jQuery to clash with respec in some way or other. None of the features (getting to the next block, sorting) works for me in Brave (which is Chromium based), on my iPad (mobile Safari based), or Firefox as far as the 'preview' version goes.
Alas! our hands are tied.
I did experiment with a much simpler solution, namely a relatively simple and standalone script that can either be added to the HTML code itself, or stored locally and linked to. That mechanism works, but it was unusable because very slow. We would have to find a very well optimized sorting script which can then be incorporated.
There was a problem hiding this comment.
Just to be on the safe side, though, I am happy to ask the powers to be whether it is acceptable to use jQuery after all. Indeed, W3C maintains on its own site, some 'frozen' copy of frameworks (the version of jQuery that is currently stored is 2.2.4). I do not know whether it is acceptable to use them in a /TR document.
There was a problem hiding this comment.
That being said, even if it is possible to use jQuery (or a specific version thereof) whether that can properly coexist with respec is a problem by itself. I may be wrong, but I seem to remember that jQuery does things on load-time, and so does respec. This means two dominating guys facing one another in the same bar...
There was a problem hiding this comment.
@iherman yes, we allow the use of scripts that are located under https://www.w3.org/scripts/. Now, we cannot ensure this won't conflict with respec and this is really a case by case basis
There was a problem hiding this comment.
@iherman yes, we allow the use of scripts that are located under https://www.w3.org/scripts/. Now, we cannot ensure this won't conflict with respec and this is really a case by case basis
Ah. This may make things a bit better. Thanks @deniak
@jricher you should try to use https://www.w3.org/scripts/jquery/2.2/jquery.min.js as the reference to jquery, to see if it works. This resolves to the 2.2.4 version.
@deniak if we badly need a newer version (I have no idea what the status of jQuery is these days) then, I presume, that can also be installed on our side, right?
There was a problem hiding this comment.
@deniak if we badly need a newer version (I have no idea what the status of jQuery is these days) then, I presume, that can also be installed on our side, right?
That should be doable. If you need a new version on /scripts, please send an email to sysreq.
There was a problem hiding this comment.
@deniak @iherman -- If I read all of the above correctly, jquery.min.js is not enough. We also need DataTables to be added to the https://www.w3.org/scripts/ space... as we cannot refer to these external files (https://cdn.datatables.net/1.10.24/css/jquery.dataTables.css and https://cdn.datatables.net/1.10.24/js/jquery.dataTables.js). Once that's done, things should work fine, as the current DataTables * @requires jQuery 1.7+
There was a problem hiding this comment.
@TallTed , I just added datatables to https://www.w3.org/scripts/. Let me know if that solves the issue.
There was a problem hiding this comment.
I expect jQuery to clash with respec in some way or other.
Although we'll try to support this in https://github.com/w3c/respec/issues/3483, I want to clarify that respec-w3c doesn't include jQuery, so there should be no clash. Though, to avoid any possible clash during the processing, consider enabling DataTable during postProcess.
Yes and no. It is no problem to use small scripts in the exported file that uses local scripts. This is the case, for example, for the JSON-LD 1.1 spec which has a small, embedded script to turn some highlights on and off. But the proposed approach is on a different scale. |
|
@iherman and others can I get a clear signal on this PR, with "changes requested" if this cannot be merged? reading the thread it seems like this solution cannot be accepted, even though it appears to work. |
I presume it works and it is only a preview problem. I so, it has to refer to https://www.w3.org/scripts/jquery/2.2/jquery.min.js and not use jquery through an external service. If that works, I have no objection using it. |
Co-authored-by: Orie Steele <[email protected]> Co-authored-by: Ted Thibodeau Jr <[email protected]>
|
I've accepted the changes to update the script locations. Note that loading on the preview is blocked by missing CORS headers on the W3C server:
|
Yes, we are discussing this to see if we can open /scripts a bit more. |
|
let me know when to review again. |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
|
Tables are still not behaving as expected/desired... The long table is displaying a sub-section, but it's only the first 10, no matter the size of the sub-section you've selected, and no matter the "page" you've selected. And there are no sort arrows/links. |
I am not sure what you mean. It all looks o.k. to me (I tried it on FF and Brave on a MacBook)
Indeed. I presume it is a CSS problem. |
|
Neither Brave, Chrome, Opera, nor Safari, all on macOS El Capitan, show me column-sort arrows, nor do they respond to changing the display count, nor to clicking a different page. To be certain we're testing the same thing, I'm going to the preview at this link. |
I am afraid that preview link did not get updated by the github app (I am not sure how it works). Try this (it is a githack preview of Justin's file). The sorting arrows are indeed not visible. But everything else seems to work. |
|
Ah, that does make everything work, though it doesn't reveal the sort arrows (which I'm afraid are rather important, so we'll need to ensure they do show up in the finished product). Great! |
|
The sort images are not being found at their expected URLs. Did those get uploaded to the W3C server as well? |
|
@TallTed @jricher I added all the missing images from https://cdn.datatables.net/1.10.24/. |
|
@deniak It looks like they still aren't being found in @iherman 's test link, I'm getting 404's on the images here: https://raw.githack.com/jricher/did-spec-registries/data-tables/index.html#did-methods |
@jricher, the CSS expected to find the images at a specific location. I've updated the CSS to point to where the images actually are. Hope this works find now. |
|
@deniak Ah, that seemed to do the trick! It took a couple tries to clear my cache but it seems to work for me. I am seeing some ReSpec errors relating to the next/previous buttons, it seems, but I'm not sure if there's much we can do about that, or if there's anything really to worry about there. |
Probably not as far as respec is concerned, although it may bother ECHIDNA; @deniak is that correct? Another question may be the accessibility aspect of the document. The HTML source has not really changed, so it should not be affected, but we may want to ask someone who knows these things better. |
The fact that there are some respec errors might break the generation (although it doesn't seem to be the case here). <div class="dataTables_paginate paging_simple_numbers" id="did-document-properties-table_paginate">
<a class="paginate_button previous disabled respec-offending-element" aria-controls="did-document-properties-table" data-dt-idx="0" tabindex="-1" id="did-document-properties-table_previous" title="Error: No matching dfn found.">Previous</a>
<span>
<a class="paginate_button current respec-offending-element" aria-controls="did-document-properties-table" data-dt-idx="1" tabindex="0" title="Error: No matching dfn found." id="respec-offender-error-no-matching-dfn-found">1</a>
</span>
<a class="paginate_button next disabled respec-offending-element" aria-controls="did-document-properties-table" data-dt-idx="2" tabindex="-1" id="did-document-properties-table_next" title="Error: No matching dfn found.">Next</a>
</div>One thing worth trying is to remove the pagination when it's not necessary. |
As far as I can see, those are not part of the original HTML source; they are added by the table software. What may help with respec is to make it sure that the table generation script runs after respec. I am not sure what the case as of now, neither do I know how to control that for somethings like the jquery tables (for shorter scripts respec gives you the possibility to say whether a script runs before or after respec has done its job). (@marcoscaceres is the one who really knows these things:-) However, if these respec error reports indeed do not bother ECHIDNA then I would propose to just ignore them... |
Pardon repeating my earlier comment, but I still find this layout hard to use. I think it would be improved by showing all methods by default. That would fix page search, the poor pagination button UX, and not hide a huge number of methods from casual readers. |
|
I would be OK with showing all entries by default, given that the "show n entries" box remains at the top of the table, as it is now. |
|
Sorry, for the delay... I would caution against adding this without thoroughly vetting if it's accessible and creates conforming HTML (apologies if you already have and it's all good.). If you'd like sortable tables cols, maybe it's something we could add directly to ReSpec? We can then include the appropriate column sorting code + ability to handle th activations, etc... possibly even add custom sorters. |
@marcoscaceres, that would be wonderful. We already had a number of cases in various specs where this would have been good (although the fact that the table in this document has ≈100 items is, I admit, an unusually high number). But if the plans in the new process towards the creation of registries as /TR documents will carry, then I expect these types of demands to become more frequent. And it would be great to have that in respec, instead of relying on the jQuery sledgehammer... |
|
Alright, filed https://github.com/w3c/respec/issues/3483 ... I'll try to carve out some time to code it up. Could we pick up the requirements there? |
|
Also sorry to "request changes" without actually providing feedback on what to change, I am mostly doing this to make it clear that my current reading of the thread is "don't do this this way, use respec". |
OR13
added
the
do not merge
I think it would be indeed better to rely on the built-in feature of respec; the version based on the jQuery library is unclear re accessibility requirements. However, we are under a time pressure. @marcoscaceres, @sidvishnoi : I know this is the question that all developers hate, but... do you think you can give us an estimate when we can rely on the new features described in https://github.com/w3c/respec/issues/3483 ? |
|
PR appears blocked by respec roadmap, suggest closing. |
|
Agree, and apologies. Just assume it's "coming soon". I'll ping once it's done. |
|
closing in 1 week, blocked by respec updates. @jricher sorry, thanks for trying.... : / |
|
still on my todo list... tryin to get to it! |
Strawman to show how DataTables could be added to the document. Currently defined on the Methods table and a new table pulling in did document properties.
Would recommend adding all properties as tables at the start of sections.
Preview | Diff