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

Updates to styling including font style and hiding GViewer toolbar buttons #6

Merged
merged 4 commits into from
Oct 1, 2015
Merged

Conversation

daveaglick
Copy link
Contributor

First off, thank you so much for making MSAGL open source, and even more for changing to an MIT license! It's a great library that fills an important niche and I think developers have been avoiding it in the past because of the murky licensing and purchasing options.

This PR is a collection of styling updates.

The biggest one is the addition of font styles. I implemented it similarly to other Node styling in that there is now a Microsoft.Msagl.Drawing.FontStyle enum instead of a direct dependency on System.Drawing. However, I only implemented support for the new font styling within the GDI control. Implementing font styling for the WPF control will be harder because there is no single property to change. In WPF, "boldness" is handled via weight, italics is handler via the WPF FontStyle static class, etc.

I also added flags to hide a couple additional GViewer toolbar buttons.

I also removed a Console.WriteLine() call in LayoutHelpers.CalculateLayout(). This was causing problems for me because in an application using MSAGL that either traps, displays, or passes on console output, these messages start to add noise. This one was appearing constantly for me. I did notice there were several other uses of Console.WriteLine(), specifically in some of the layout engines - maybe those should be removed as well and a logging infrastructure put in place instead?

@msftclas
Copy link

msftclas commented Apr 8, 2015

Hi @daveaglick, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@levnach
Copy link
Contributor

levnach commented Apr 8, 2015

Dave, thank you for the PR! I have one concern. I suspect that after your change old versions of MSAGL files would not load. Please use GraphReader.TokenIs method to check that the FontStyle XML node is present in the file that is being loaded, and if it is not present then the label should be created with the default style.
I agree with your comment on WriteLine()

@daveaglick
Copy link
Contributor Author

That makes perfect sense. Just committed a patch (hopefully I used GraphReader.TokenIs() correctly).

Also, it may be a day or two before I have a signed CLA ready for you. My employer's legal department has requested to review it.

@birbilis
Copy link

birbilis commented Jun 3, 2015

So, does the MSBOT detect when CLA signing has been done and update such pending requests with that info/label?

@rassilon
Copy link

rassilon commented Jun 3, 2015

The MSBOT does detect when the CLA signing occurs as long as you fill out the information correctly since it is related to your github account, IIRC. My open PRs got turned into cla-signed from cla-required. New PRs of mine now get cla-already-signed.

Fyi,
Bill

@daveaglick
Copy link
Contributor Author

Yeah, about that... I actually haven't signed the CLA yet, and at this point I suspect I'll need to do a merge on my PR to get it up to date before it's accepted.

Sorry for the delay - unfortunately, the CLA requires employer participation and mine is currently "reviewing" the agreement (and has been for a while). I have no idea when I'll get the go-ahead to sign, though I've at least been told they'll support it eventually.

Once I do get it signed, I'll do a quick catch-up on the PR.

@birbilis
Copy link

birbilis commented Jun 3, 2015

btw, regarding Console.WriteLine, wasn't there a Debug.WriteLine that gets ignored at production builds?

https://support.microsoft.com/en-us/kb/815788
says "If the solution configuration type is Release, the Debug class output is ignored."

@msftclas
Copy link

msftclas commented Oct 1, 2015

@daveaglick, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@daveaglick
Copy link
Contributor Author

"Thanks for signing the contribution license agreement so quickly" Lol.

It's been long enough that my branch is now way behind. I'm working on a rebase and should have this PR updated soon.

@daveaglick
Copy link
Contributor Author

Okay, should be good to go now.

@levnach
Copy link
Contributor

levnach commented Oct 1, 2015

Hi Dave, I am getting this error when building your sources.

C:\dev\git\GraphLayout\tools\GraphmapsWpfControl\GraphmapsViewer.cs(2079,42,2079,51): error CS0104: 'FontStyle' is an ambiguous reference between 'System.Windows.FontStyle' and 'Microsoft.Msagl.Drawing.FontStyle'

@daveaglick
Copy link
Contributor Author

Bummer. Might be something with the rebase. My changes don't touch the WPF control (and so I had it unloaded), but perhaps Git did a bad job with the automatic merge. I'll take a look and correct tomorrow, thanks.

levnach added a commit that referenced this pull request Oct 1, 2015
Updates to styling including font style and hiding GViewer toolbar buttons
@levnach levnach merged commit 2e7f3c0 into microsoft:master Oct 1, 2015
@daveaglick
Copy link
Contributor Author

Thanks!

@levnach
Copy link
Contributor

levnach commented Oct 1, 2015

I fixed the build of WPF stuff. Thanks!

levnach pushed a commit that referenced this pull request Aug 24, 2017
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.

5 participants