Skip to content

box_shadow example with adjustable settings #19345

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

Merged
merged 9 commits into from
May 27, 2025

Conversation

oracle58
Copy link
Contributor

@oracle58 oracle58 commented May 23, 2025

Objective

  • Addresses the previous example's lack of visual appeal and clarity. It was missing labels for clear distinction of the shadow settings used on each of the shapes. The suggestion in the linked issue was to either just visually update and add labels or to collapse example to a single node with adjustable settings.
  • Fixes Improve the box_shadow example #19240

Solution

  • Replace the previous static example with a single, central node with adjustable settings as per issue suggestion.
  • Implement button-based setting adjustments. Unfortunately slider widgets don't seem available yet and I didn't want to further bloat the example.
  • Improve overall aesthetics of the example -- although color pallette could still be improved. flat gray tones are probably not the best choice as a contrast to the shadow, but the white border does help in that aspect.
  • Dynamically recolor shadows for visual clarity when increasing shadow count.
  • Add Adjustable Settings:
    • Shape selection
    • Shadow X/Y offset, blur, spread, and count
  • Add Reset button to restore default settings

The disadvantage of this solution is that the old example code would have probably been easier to digest as the new example is quite bloated in comparison. Alternatively I could also just implement labels and fix aesthetics of the old example without adding functionality for adjustable settings, but I personally feel like interactive examples are more engaging to users.

Testing

  • Did you test these changes? If so, how? cargo run --example box_shadow and functionality of all features of the example.
  • Are there any parts that need more testing? Not that I am aware of.
  • How can other people (reviewers) test your changes? Is there anything specific they need to know? Not really, it should be pretty straightforward just running the new example and testing the feats.

Showcase

box-shadow-example-1

box-shadow-example-2

@oracle58 oracle58 force-pushed the improve-box-shadow branch from 792458e to a1e8f7c Compare May 23, 2025 13:08
- Replaces the previous static example with a single, central node with adjustable settings as per issue suggestion.
- Implements button-based setting adjustments. Slider widgets don't seem available yet.
- Improved overall aesthetics of the example
- Dynamically recolors shadows for visual clarity when increasing shadow count.
- Adjustable Settings:
    - Shape selection
    - Shadow X/Y offset, blur, spread, and count
- Added Reset button to restore default settings
@oracle58 oracle58 force-pushed the improve-box-shadow branch from a1e8f7c to 91b39f8 Compare May 23, 2025 13:08
@oracle58 oracle58 changed the title Interactive, single node box_shadow example box_shadow example with adjustable settings May 23, 2025
@ickshonpe ickshonpe self-requested a review May 23, 2025 17:40
Copy link
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

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

Really nice. Just made a few suggestions but overall it's a huge improvement over the old example.

@oracle58 oracle58 force-pushed the improve-box-shadow branch from 442c267 to 7a18fa5 Compare May 24, 2025 11:21
@oracle58
Copy link
Contributor Author

Really nice. Just made a few suggestions but overall it's a huge improvement over the old example.

Thanks for the feedback and review! Suggestions already applied for the most part. Just working out the button pressed repeat logic.

oracle58 added 2 commits May 24, 2025 14:08
make shape label transparent while keeping others gray for improved UI clarity
- Track last pressed button and press time in a resource
- Add system to trigger button actions at a repeat interval when held
- Improves UX for increment/decrement and shape navigation buttons
@oracle58 oracle58 force-pushed the improve-box-shadow branch from 68dc3e1 to cafaf79 Compare May 24, 2025 17:30
@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples A-UI Graphical user interfaces, styles, layouts, and widgets S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 26, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Very fancy! I'm looking forward to replacing this complexity with some native widgets in the future, but I think this is a step in the right direction.

IMO it's much more important for this example to allow users to explore the box shadow options interactively, rather than simply learning how to add a box shadow.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 26, 2025
@oracle58
Copy link
Contributor Author

Very fancy! I'm looking forward to replacing this complexity with some native widgets in the future, but I think this is a step in the right direction.

Absolutely, native widgets and possibly even a template for interactive examples using some set of these widgets would definitely reduce complexity and furthermore enable consistency across the examples in the long-term. Especially if the objective is to make more examples interactive down the road.

IMO it's much more important for this example to allow users to explore the box shadow options interactively, rather than simply learning how to add a box shadow.

I also feel that is the right choice.

@JMS55
Copy link
Contributor

JMS55 commented May 26, 2025

The downside is that the CI screenshot tests no longer tests all the different shadow combinations

@alice-i-cecile
Copy link
Member

@oracle58 ping me when merge conflicts are resolved and I'll get this in :)

Copy link
Contributor

@rparrett rparrett left a comment

Choose a reason for hiding this comment

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

IMO, this and other example widgetification attempts are a bit premature. This is adding a lot of example code that must be maintained and is likely to change soon.

If we want to do this, I have some suggested changes.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels May 26, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone May 26, 2025
oracle58 added 2 commits May 27, 2025 14:36
- rename `spawn_setting_children` -> `build_setting_row`
- remove bg color from value labels (made them seem like input fields)
- Fix settings panel positioning: bottom and left margins between screen and panel are uneven
- use `resource_changed` rather than `is_changed` for shape and shadow settings
- The `button_color_system` now uses With<SettingsButton> in its query filter, so it only runs for buttons with a `SettingsButton` component.
- Refactor setting value updates to use new `SettingType` enum component instead of fragile label string comparisons
- Add Samples as a controllable setting in the UI panel
@oracle58 oracle58 requested a review from rparrett May 27, 2025 12:41
@oracle58 oracle58 force-pushed the improve-box-shadow branch from a55ca05 to 5394519 Compare May 27, 2025 12:58
- Replace tuple struct syntax with BorderColor::all(...) in shape node and settings panel.
- Resolves E0423 error due to struct construction changes in Bevy UI.
- consistent formatting
- Fmt imports
@oracle58 oracle58 force-pushed the improve-box-shadow branch from 5394519 to e1d7b2c Compare May 27, 2025 13:04
@oracle58
Copy link
Contributor Author

If we want to do this, I have some suggested changes.

I think I have the suggested changes applied @rparrett

@oracle58 ping me when merge conflicts are resolved and I'll get this in :)

@alice-i-cecile
Conflicts are resolved and suggestions from @rparrett applied.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented May 27, 2025

IMO, this and other example widgetification attempts are a bit premature. This is adding a lot of example code that must be maintained and is likely to change soon.

I 100% see where you're coming from, but I'd like to merge a couple of these now, mostly so we can compare them as a baseline.

Thanks for the thorough feedback as always :)

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 27, 2025
Merged via the queue into bevyengine:main with commit 8e58517 May 27, 2025
32 checks passed
github-merge-queue bot pushed a commit that referenced this pull request May 28, 2025
# Objective

Minimal effort to address feedback here:
#19345 (comment)
more thoroughly.

## Solution

- Remove hardcoded label string comparisons and make more use of the new
enum added during review
- Resist temptation to let this snowball this into a huge refactor
- Maybe come back later for a few other small improvements

## Testing

`cargo run --example box_shadow`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Examples An addition or correction to our examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the box_shadow example
5 participants