Skip to content
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

Common widget GzColor replacement #1530

Merged
merged 18 commits into from
Aug 16, 2022
Merged

Conversation

AzulRadio
Copy link
Contributor

@AzulRadio AzulRadio commented Jun 8, 2022

New feature

Summary

Replace implementation for lights (Lights.qml) and material color (Material.qml) in Component Inspector with GzColor introduced in gz-gui#411

For future maintainers: selecting color with the Material panel has a bug where the color of the material doesn't change when a color is selected. This bug is not introduced by this common widget.

Test it

demo_fortress_lightgif

Draft because of a possible bug in the Gazebo which causes the colors of the lights and material color can only be changed once.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@AzulRadio AzulRadio changed the title Common widget GzColor for Lights Common widget GzColor replacement Jun 8, 2022
@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #1530 (7b52abf) into ign-gazebo6 (8c9489d) will increase coverage by 0.09%.
The diff coverage is 0.00%.

@@               Coverage Diff               @@
##           ign-gazebo6    #1530      +/-   ##
===============================================
+ Coverage        64.40%   64.50%   +0.09%     
===============================================
  Files              320      320              
  Lines            25892    25861      -31     
===============================================
+ Hits             16677    16682       +5     
+ Misses            9215     9179      -36     
Impacted Files Coverage Δ
.../plugins/component_inspector/ComponentInspector.cc 5.61% <0.00%> (+0.28%) ⬆️
src/Util.cc 93.11% <0.00%> (-0.33%) ⬇️
src/SimulationRunner.cc 91.89% <0.00%> (+0.95%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chapulina chapulina requested a review from jennuine June 13, 2022 15:56
@chapulina chapulina added 🏯 fortress Ignition Fortress OOBE 📦✨ Out-of-box experience labels Jun 13, 2022
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Jun 22, 2022
@AzulRadio AzulRadio removed the needs upstream release Blocked by a release of an upstream library label Jul 19, 2022
@jennuine jennuine added the needs upstream release Blocked by a release of an upstream library label Jul 21, 2022
@jennuine
Copy link
Contributor

This PR needs a release from gz-gui6 but I'm waiting until the plotting PR is ready and merged (ref: #1571 (comment))

@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Aug 4, 2022
@AzulRadio AzulRadio marked this pull request as ready for review August 10, 2022 21:10
@AzulRadio AzulRadio added the GUI Gazebo's graphical interface (not pure Ignition GUI) label Aug 10, 2022
Signed-off-by: Jenn Nguyen <[email protected]>
Signed-off-by: Jenn Nguyen <[email protected]>
Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor comment

src/gui/plugins/component_inspector/Light.qml Outdated Show resolved Hide resolved
Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

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

Actually, here's a couple more comments regarding spacing

src/gui/plugins/component_inspector/Light.qml Outdated Show resolved Hide resolved
src/gui/plugins/component_inspector/Light.qml Show resolved Hide resolved
src/gui/plugins/component_inspector/Material.qml Outdated Show resolved Hide resolved
@AzulRadio AzulRadio merged commit 4d99cfc into ign-gazebo6 Aug 16, 2022
@AzulRadio AzulRadio deleted the azulradio/common_widget_color branch August 16, 2022 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress GUI Gazebo's graphical interface (not pure Ignition GUI) OOBE 📦✨ Out-of-box experience
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants