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

Add ScrollLog with TypeScript #5237

Closed
wants to merge 6 commits into from
Closed

Add ScrollLog with TypeScript #5237

wants to merge 6 commits into from

Conversation

ahuang11
Copy link
Contributor

@ahuang11 ahuang11 commented Jul 9, 2023

Refactors #5212 so it's implemented as a Typescript subclass of the Bokeh Column based on the this suggestion #5212 (comment)

I understand that Marc would like to add "actions" like AutoScroll FullScreen, Sort, etc. to a Column, but first I'd like to figure out how to translate the ReactHTML into Typescript as a draft first.

I modeled this after pn.Card, but I'm missing something related to registration:

Bokeh items: Error: could not resolve type 'panel.models.layout.ScrollLog', which could be due to a widget or a custom model not being registered before first usage

Even after running python setup.py develop and panel build panel

@hoxbro
Copy link
Member

hoxbro commented Jul 9, 2023

I think you are missing adding it to index.ts

A PR for inspiration could be #4909 as it is very limited in scope and implements a new model.

@ahuang11
Copy link
Contributor Author

ahuang11 commented Jul 9, 2023

Thanks for sharing that; I'll definitely use that as reference.

I added index.ts, but it's still showing the same thing; maybe an error in my Typescript.

@codecov
Copy link

codecov bot commented Jul 9, 2023

Codecov Report

Merging #5237 (df057ce) into main (f7f9ea9) will decrease coverage by 13.63%.
The diff coverage is 92.85%.

@@             Coverage Diff             @@
##             main    #5237       +/-   ##
===========================================
- Coverage   83.82%   70.19%   -13.63%     
===========================================
  Files         274      275        +1     
  Lines       39594    39619       +25     
===========================================
- Hits        33189    27812     -5377     
- Misses       6405    11807     +5402     
Flag Coverage Δ
ui-tests ?
unitexamples-tests 70.19% <92.85%> (-3.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
panel/__init__.py 100.00% <ø> (ø)
panel/layout/scrolllog.py 90.00% <90.00%> (ø)
panel/layout/__init__.py 100.00% <100.00%> (ø)
panel/models/__init__.py 100.00% <100.00%> (ø)
panel/models/layout.py 100.00% <100.00%> (ø)

... and 95 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@philippjfr
Copy link
Member

Also to be fully clear the suggestion was to swap out the model that renders Column and then add an option to add the scroll features, so we don't add yet another layout.

@ahuang11
Copy link
Contributor Author

I still can't seem to register it to get a testable version :(

Error rendering Bokeh items: Error: could not resolve type 'panel.models.layout.ScrollLog', which could be due to a widget or a custom model not being registered before first usage
    at p.error (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:4951)
    at p._resolve_type (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:5059)
    at p._decode_object_ref (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:4782)
    at p._decode (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:1779)
    at bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:2249
    at f (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:178:450)
    at p._decode_plain_array (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:2237)
    at p._decode (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:928)
    at bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:614
    at p.decode (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:211:730)

@philippjfr
Copy link
Member

Did you try shift-reload? Are you testing in a notebook?

@ahuang11
Copy link
Contributor Author

Oh shift-reload changed the error. Okay, so I think I now can test.

Error rendering Bokeh items: TypeError: Cannot read properties of undefined (reading 'addEventListener')
    at r.connect_signals (panel.min.js?v=3d8aca08eab1e0268df3670e2d168569f81cfcdfaac4320304acbd95653e5732:143:477)
    at t.build_view (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:224:342)
    at async bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:218:236
    at async f (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:218:198)
    at async t.add_document_standalone (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:218:459)
    at async g (bokeh.min.js?v=c8d721b75531cff043feef626ae0d6e29a26031fae26bba93c1baa962ecfb1b43f211027df3a0cf7772f30afed8c38198d70677b8967e581ca7e605358c68465:163:804)

@ahuang11
Copy link
Contributor Author

Woohoo got it to work. Will try to generalize it to a new PanelColumn model next

Screen.Recording.2023-07-10.at.4.59.24.PM.mov

@ahuang11
Copy link
Contributor Author

Here's my attempt to extend BkColumn.
https://github.com/holoviz/panel/compare/column_replacement?expand=1

I think there's an issue with name conflict because I get this warning/error in VSCode

File name '/Users/ahuang/repos/panel/panel/node_modules/@bokeh/bokehjs/build/js/lib/models/layouts/Column.d.ts' differs from already included file name '/Users/ahuang/repos/panel/panel/node_modules/@bokeh/bokehjs/build/js/lib/models/layouts/column.d.ts' only in casing.
  The file is in the program because:
    Imported via "./column" from file '/Users/ahuang/repos/panel/panel/node_modules/@bokeh/bokehjs/build/js/lib/models/layouts/index.d.ts' with packageId '@bokeh/bokehjs/build/js/lib/models/layouts/column.d.ts@3.2.0'
    Imported via "@bokehjs/models/layouts/column" from file '/Users/ahuang/repos/panel/panel/models/card.ts' with packageId '@bokeh/bokehjs/build/js/lib/models/layouts/column.d.ts@3.2.0'
    Imported via "@bokehjs/models/layouts/Column" from file '/Users/ahuang/repos/panel/panel/models/column.ts' with packageId '@bokeh/bokehjs/build/js/lib/models/layouts/Column.d.ts@3.2.0'ts(1149)
index.d.ts(1, 24): File is included via import here.
card.ts(1, 34): File is included via import here.
module "/Users/ahuang/repos/panel/panel/node_modules/@bokeh/bokehjs/build/js/lib/models/layouts/column"
No quick fixes available

@philippjfr
Copy link
Member

Can you push your changes?

@ahuang11
Copy link
Contributor Author

Yep; it's on this branch:
column_replacement
#5245

@ahuang11 ahuang11 changed the title TypeScript ScrollLog Add ScrollLog with TypeScript Jul 11, 2023
@ahuang11
Copy link
Contributor Author

Closed in favor of #5245

@ahuang11 ahuang11 closed this Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants