Skip to content

Conversation

trungleduc
Copy link
Member

No description provided.

@trungleduc trungleduc added the enhancement New Feature or improvement label Sep 17, 2025
Copy link
Contributor

Integration tests report: appsharing.space

Copy link
Contributor

Preview PR at appsharing.space

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

Thanks!

LGTM, I only have some concerns about that filepath parameter, and we should make all the labels prettier since they are user facing

const { commands, tracker, trans } = options;

commands.addCommand(ShapeCreationCommandIDs.newBoxWithParams, {
label: trans.__('New Box From Parameters'),
Copy link
Member

Choose a reason for hiding this comment

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

Since this is user facing, should we make this more like:

Suggested change
label: trans.__('New Box From Parameters'),
label: trans.__('Create a box'),

Copy link
Member Author

@trungleduc trungleduc Sep 18, 2025

Choose a reason for hiding this comment

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

No, these commands are being used internally; users can't invoke them since there is no way to provide the required parameters

Copy link
Member Author

Choose a reason for hiding this comment

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

we have user-facing commands to gather input then call these commands to do the job

@trungleduc
Copy link
Member Author

I updated some parameter descriptions and default values

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

Thanks!

@trungleduc trungleduc merged commit fee0d2a into jupytercad:main Sep 18, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New Feature or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants