wave icon indicating copy to clipboard operation
wave copied to clipboard

Add height attribute to ui.copyable_text if multiline is True

Open mturoci opened this issue 4 years ago • 1 comments

Discussed in https://github.com/h2oai/wave/discussions/1302

Originally posted by azim-b March 22, 2022 can we add a height param to the ui.copyable_text() component?

Example use case:

  • text box of certain height
  • user enters text and hits Submit
  • if the query was successfully, user gets back a "copyable text box" of same height (i.e. ui.copyable_text() of same height)

Description

Allow controlling height of the ui.copyable_text if multiline is set to True. Should work the same as ui.textbox.

mturoci avatar Mar 24 '22 08:03 mturoci

Hey @mturoci, can I take this issue? I haven't worked with UI specifically for python before but I am experienced in the language. Any additional guidelines would be very helpful!

scharf-roie avatar Sep 21 '22 17:09 scharf-roie

Hi @scharf-roie, definitely go for it! However, note that the work needed to be done is going to be in React/Typescript mostly - https://github.com/h2oai/wave/blob/master/ui/src/copyable_text.tsx.

Please go through our contributing guide and set up your dev env first. Should you have any questions, do not hesitate to ask on our Discord :)

mturoci avatar Sep 22 '22 06:09 mturoci

Since 4 days have passed already, the issue is up for grabs again.

mturoci avatar Oct 03 '22 10:10 mturoci

Hi @mturoci, I am a student looking to contribute to Wave. This task seems like a good starting point, do you think I could take this issue?

I propose a simple change to ui.copyable_text.jsx, wherein we add a height property to the copyable_text interface, add height to the model, and change this line: <Fluent.TextField componentRef={ref} value={value} label={label} multiline={multiline} styles={{ root: { width: pc(100) } }} readOnly /> to this line: <Fluent.TextField componentRef={ref} value={value} label={label} multiline={multiline} styles={ multiline ? { root: { width: pc(100), height: height} } : { root: { width: pc(100) } } } readOnly />

Also, I plan to add test case(s) for this, but I'm not sure I understand the current tests just by looking at them. Is there some resource for writing TypeScript tests for Wave that you could point me to?

cvanvoorhis avatar Nov 10 '22 22:11 cvanvoorhis

Hi @cvanvoorhis, please go ahead and create a PR.

not sure I understand the current tests

Wave has 2 types of tests currently:

  • Unit tests (*.test.tsx) - these test the component interactions and business logic. Your proposed change doesn't do either of these.
  • Visual tests - screenshot px by px comparison for visual changes - this one is the way to go for this task. All you need to do is add a working example into copyable_text.md.

mturoci avatar Nov 11 '22 07:11 mturoci

Thank you @mturoci! Been a little busy so I'm planning to get a PR up tomorrow when I have more time.

cvanvoorhis avatar Nov 12 '22 16:11 cvanvoorhis