Skip to content

fix dat.GUI button elements layout - #100

Merged
rdeits merged 1 commit into
meshcat-dev:masterfrom
RussTedrake:long_button_names
Sep 7, 2021
Merged

rdeits merged 1 commit into
meshcat-dev:masterfrom
RussTedrake:long_button_names

Conversation

@RussTedrake

Copy link
Copy Markdown
Collaborator

without this fix, buttons added to the GUI with a long name were artificially truncated to the same length that a slider name gets truncated (to make room for the slider and text input). The button should not have that restriction.

After this fix, the complete long name of the button is displayed properly.

@RussTedrake
RussTedrake requested a review from rdeits September 5, 2021 11:49
Comment thread test/controls.html
// the callback was not called.
viewer.set_control_value("TestSlider", .4, false);

viewer.set_control("Test button with a really long name", () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this file to include main.min.js? It currently uses main.js which isn't tracked in git

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (I had copied it from empty_scene.html)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

without this fix, buttons added to the GUI with a long name were artificially truncated to the same length that a slider name gets truncated (to make room for the slider and text input).  The button should not have that restriction.

After this fix, the complete long name of the button is displayed properly.
@RussTedrake

Copy link
Copy Markdown
Collaborator Author

FWIW, I've also gone ahead and PR'd the upstream fix. dataarts/dat.gui#306 . If that goes through, then we can bump the dependency and get rid of the one-line bandaid. But I would like to land this for now.

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.

2 participants