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

UI improvements in symbol layer widgets #220

Merged
merged 1 commit into from Oct 11, 2012
Merged

UI improvements in symbol layer widgets #220

merged 1 commit into from Oct 11, 2012

Conversation

tecoholic
Copy link
Contributor

Hello @NathanW2, @wonder-sk and @etiennesky

I have made a number of UI changed in the Symbol layer properties widgets.

  • Mostly corrected the alignment of the parameter input fields to be in line with the top Layer Type combo.
  • Then removed a few buttons which were meant to call Symbol Editor for sub-symbols.
  • Tweaked the color selection buttons so that they show color on themselves properly.

Test it out and merge if you like, if there is anything bothering do let me know. I am chipping away things little be little expect small updates now and then.

@etiennesky
Copy link
Contributor

Arun - I just noticed that color ramps (but not symbols) are duplicated in the database when you save an existing symbol. Can you have a look please?

@tecoholic
Copy link
Contributor Author

@etiennesky I missed the UNIQUE keyword in the symbol_xml2db.py script for colorramp names. Look at https://github.com/qgis/Quantum-GIS/blob/master/scripts/symbol_xml2db.py#L39 . Could you add that and recreate the DB for me please.

Update: But then, there is something to fix in the code as well. I will fix it and let you know.

@tecoholic
Copy link
Contributor Author

Ok, found the bug. In line 191 here-> https://github.com/qgis/Quantum-GIS/blob/master/src/core/symbology-ng/qgsstylev2.cpp#L191 , the mSymbols.contains() is supposed to be mColorRamps.contains() or colorRampNames().contains(). That fixes it. I hope you can do that :)

@etiennesky
Copy link
Contributor

Hi, will do both, thanks for the fixes.

@etiennesky
Copy link
Contributor

I'll let others review this request, because I can't understand the differences and don't want to interfere.

@tecoholic
Copy link
Contributor Author

@NathanW2 and @wonder-sk : Just a reminder, this pull request has been long standing. I guess you both are busy, otherwise it would have been evaluated long ago, I just wanted to remind you to consider this when you find enough free time.

@wonder-sk
Copy link
Member

I have been on vacation last four weeks - I should be able to look at it
during the next week - please ping me if I forget!

@NathanW2
Copy link
Member

@tecoholic sorry man just got busy and didn't get a chance to take a look. Slowing getting back into the grove of things.

@ghost ghost assigned wonder-sk Oct 6, 2012
wonder-sk added a commit that referenced this pull request Oct 11, 2012
UI improvements in symbol layer widgets
@wonder-sk wonder-sk merged commit def6486 into qgis:master Oct 11, 2012
@wonder-sk
Copy link
Member

Changes look good - merged - and thanks for your patience!

yjacolin pushed a commit to yjacolin/QGIS that referenced this pull request Oct 4, 2014
Copyediting review through user_manual/grass_integration/
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

4 participants