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

[symbology] migrate graduated renderer color ramp widget #3815

Merged
merged 1 commit into from
Nov 30, 2016

Conversation

nirvn
Copy link
Contributor

@nirvn nirvn commented Nov 30, 2016

This PR migrates the graduated renderer from the deprecated color ramp combo box to the new color ramp button. I've also taken the opportunity to clean up the layout a bit (i.e. look at the now-aligned "color ramp" label, etc.).

The end result in one screenshot:
untitled

Even with the addition of code to deal with the layout improvements, the PR results in fewer lines of code, thanks to the invert logic moved away from UI / renderer into the color ramp itself.

@nyalldawson , as always, your keen eye is most welcome.

@nirvn
Copy link
Contributor Author

nirvn commented Nov 30, 2016

@nyalldawson , this PR is green, the red comes from (newly-added / unwanted ?) AppVeyor's unrelated failure.

@NathanW2
Copy link
Member

Quick scan looks good to me.

@nirvn
Copy link
Contributor Author

nirvn commented Nov 30, 2016

@NathanW2 , thanks. I'll take that as my second green light to merge a PR today 😄

@nirvn nirvn merged commit ae75e45 into qgis:master Nov 30, 2016
@nyalldawson
Copy link
Collaborator

@nirvn - looks good to me to, but don't forget the api break docs

@nirvn
Copy link
Contributor Author

nirvn commented Nov 30, 2016

@nyalldawson , good point. I'm planning to do that when I get rid of the QgsColorRampComboBox class after migration is complete.

bstroebl pushed a commit to bstroebl/QGIS that referenced this pull request Dec 1, 2016
@nirvn nirvn deleted the colorrampbutton_migration branch December 28, 2016 04:19
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.

None yet

3 participants