Skip to content

library: Add Box entry#216

Merged
sonnyp merged 8 commits intoworkbenchdev:mainfrom
AkshayWarrier:add-box-entry
Mar 13, 2023
Merged

library: Add Box entry#216
sonnyp merged 8 commits intoworkbenchdev:mainfrom
AkshayWarrier:add-box-entry

Conversation

@AkshayWarrier
Copy link
Copy Markdown
Contributor

This PR attempts to add an interactive demo for Gtk.Box

Issue workbenchdev/demos#3

Copy link
Copy Markdown
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

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

This looks good and I especially like the UX of the slider to change the alignment.

Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.css Outdated
Comment thread src/Library/demos/Box/main.css
Comment thread src/Library/demos/Box/main.css Outdated
Comment thread src/Library/demos/Box/main.js Outdated
Comment thread src/Library/demos/Box/main.js Outdated
Copy link
Copy Markdown
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Awesome to get an interactive Box demo 👍

Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.blp Outdated
Comment on lines +58 to +60
CheckButton toggle_orient {
label: "Toggle Orientation";
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use the the name of the property and the name of the possible values so it clears to users what we are changing here.

orientation: vertical|horizontal

I'd suggest using grouped toggle buttons (see #219)

Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.blp Outdated
Comment thread src/Library/demos/Box/main.blp Outdated
@AkshayWarrier AkshayWarrier marked this pull request as draft March 12, 2023 15:39
@AkshayWarrier AkshayWarrier marked this pull request as ready for review March 12, 2023 22:42
@AkshayWarrier AkshayWarrier requested a review from sonnyp March 12, 2023 22:43
@sonnyp sonnyp requested a review from andyholmes March 12, 2023 23:34
Copy link
Copy Markdown
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

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

This really great for demonstrating both GtkBox and alignment/expansion, well done!

Just a few layout tweaks and I think this is good to go if @sonnyp agrees.

Comment thread src/Library/demos/Box/main.blp
Comment thread src/Library/demos/Box/main.blp
Comment thread src/Library/demos/Box/main.blp
Copy link
Copy Markdown
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

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

Great job!

Copy link
Copy Markdown
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Fantastic 👍

I added a couple of tooltips to clarify what "halign" / "valign" stands for

6c984a1

@sonnyp sonnyp merged commit 00be084 into workbenchdev:main Mar 13, 2023
@AkshayWarrier
Copy link
Copy Markdown
Contributor Author

Thanks! 😄

@AkshayWarrier AkshayWarrier deleted the add-box-entry branch March 15, 2023 03:19
sonnyp pushed a commit to SoNiC-HeRE/Workbench that referenced this pull request Mar 21, 2023
andyholmes pushed a commit to andyholmes/Workbench that referenced this pull request Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants