-
-
Notifications
You must be signed in to change notification settings - Fork 72
Conversation
- change reset css to allow markdown styling
0776560
to
c8f31bd
Compare
fe1cab8
to
9972df4
Compare
This preserves alignment for code and lists.
b69da8a
to
e7f63fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Navigation works well when the user
single clicks
on the cell, on adouble click
the focus goes back to the page's body and navigation inside the table becomes impossible -
Seeing quite a few percy diffs for fixed columns / rows test cases
Very nice visual tests 😄
} | ||
|
||
export default class CellMarkdown extends PureComponent<IProps> { | ||
private static md: Remarkable = new Remarkable(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking that maybe we can include the configuration for highlight.js usage, externalize the resource and expect special code block users to include the highlight.js they want as a resource from https://highlightjs.org/download/ ?! Either that or (1) not support it at all or (2) asyncing the resource -- in which case React.Suspense
won't do and we'll have to handle it through state on CellMarkdown
. This is mostly about completeness.. we should not load JS by default as it will involve a very special use case.
private static md: Remarkable = new Remarkable({
highlight: (value: string, language: string) => {
const hljs: any = (window as any).hljs;
if (!hljs) {
return '';
}
if (language && hljs.getLanguage(language)) {
try {
return hljs.highlight(language, value).value;
} catch (_) { }
}
try {
return hljs.highlightAuto(value).value;
} catch (_) { }
return '';
}
});
I don't think we should flat out not support highlighting and I don't think this will ever be enough of a priority on its own to be fixed later, if possible I think I'd rather get it in w/ async and wrap it up.
7a22e21
to
baff033
Compare
third-party/highlight.js
Outdated
highlightjs.registerLanguage('xml', xml); | ||
highlightjs.registerLanguage('yaml', yaml); | ||
|
||
export default highlightjs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is now our own cherry-picked version of Highlight.js would rather see this somewhere inside of /src
, even if it's just /src/third-party/Highlight.js
or some variation.
@@ -23,14 +23,14 @@ declare module 'fast-isnumeric' { | |||
} | |||
|
|||
declare class Remarkable { | |||
constructor(); | |||
constructor(options?: any); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
render(value: string): any; | ||
} | ||
|
||
declare interface RemarkableCtor { | ||
new(): Remarkable; | ||
new(options?: any): Remarkable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that with @types/Remarkable
added, all of the remarkable definitions becomes redundant.
tslint.json
Outdated
@@ -9,6 +9,7 @@ | |||
"cypress/**", | |||
"inst/**", | |||
"node_modules/**", | |||
"third-party/**", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now redundant
234fe22
to
fc63409
Compare
86f0485
to
a2af831
Compare
@shammamah Testing navigation with:
Notice that for both tables it's possible to click/select a markdown cell and keyboard/navigate within the markdown column as expected. The top table, with |
hi Can anyone help me here ? I am totally stuck. Trying to embedd image from link into dash table cell. Following is the code snippet. |
Fixes #596
check virtualized performance
virtualized behavior (varying row height -- breaks assumptions, this will be dealt outside the scope of this PR as this behavior is not caused by MD)
varying row height with fixed columns (Using DataTable
row_selectable
androw_deletable
with fixed columns results in mismatched row heights #649)navigation through and from markdown cell
define copy/paste behavior (?!)
define filter behavior -- for now filters will behave the same way for markdown cells as for any other text cells (the value filtered on is the actual value in the df) -- expect at least one visual/sanity test for the filter behavior (guard rail)
define sort behavior (supporting/using displayed values is out of scope) -- for now sort will behave the same way for markdown cells as for any other text cells (the value sorted on is the actual value in the df) -- expect at least one visual/sanity test for the sort behavior (guard rail)
visual tests -- cover as much of md syntax as possible
behavior on non-text input value + test (e.g. number, boolean values, how does ReMarkable behave)
markdown image resize (test, docs)
more tests
docs (will be done in https://github.com/plotly/dash-docs/pull/733)
Test: Default highlight.js included with table does not assign itself to
window.hljs
Test: Loading external highlight.js will take precedence on the table's version