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

Adding background to features in word - fixed #58

Merged
merged 4 commits into from
Aug 10, 2013
Merged

Adding background to features in word - fixed #58

merged 4 commits into from
Aug 10, 2013

Conversation

jni-
Copy link

@jni- jni- commented Aug 9, 2013

This is a naive attempt at modifying the word generator to include backgrounds. I'm not sure it's the best, so you may want to adjust styling and such - sorry if it's not that elegant, I had never generated word documents from c#.

Here is the result (in libreoffice, you may want to try it in word... I'll give it a try in word tomorrow)

2013-08-08_23-24_628x274

@dirkrombauts
Copy link
Member

Thanks. I never generated Word from C# either, so I'm not in a position to comment on the elegance of your code ;-)

I like what you did, the visuals are fine for me too. I have a couple of questions/remarks, though. Specifically:

  1. The background box takes its title from the Name of the Scenario (WordBackgroundFormatter.CS, line 18). Any special reason for that? SpecFlow simply uses the "Background" keyword.
  2. The table of content now contains an entry for the Background too. Is this by design? I'm undecided whether I like it.
  3. I like to complain about this: WordBackgroundFormatter.cs contains "using" statements that are not needed. Could you please remove them? ReSharper can do that for you, and I believe the newest versions of Visual Studio have a "order usings" feature as well.

Don't get me wrong, I am grateful for your work of bringing the Background thing to the Word document format.

BTW: there's no need to close this pull request and create a new one with your modifications. Just leave this one open and when you create a new pull request, github will guide you in updating this request.

@ghost ghost assigned dirkrombauts Aug 9, 2013
@jni-
Copy link
Author

jni- commented Aug 9, 2013

Thanks for the feedback!

1- You're right, my bad. I fixed this with the localized keyword for background.
2- That's merely a side effect of using the "Heading2" style. Maybe we should just make the text bold?
3 - I forgot to set my resharper properly at home. Usually it puts dead code in bright orange so I can't miss it, my bad!

I'm wondering if adding a description to a background is permitted. If so, should I add that too?

@dirkrombauts
Copy link
Member

About the table of content: let's leave it as it is now.

Description in background: I just checked and yes, SpecFlow allows it. So we should add that too. Good catch!

@jni-
Copy link
Author

jni- commented Aug 9, 2013

Alright I'll add this tonight when I get back from work! Thanks for checking

@jni-
Copy link
Author

jni- commented Aug 9, 2013

There you go. Here's the update screenshot :

2013-08-09_16-11_606x377

dirkrombauts added a commit that referenced this pull request Aug 10, 2013
Adding background to features in word
@dirkrombauts dirkrombauts merged commit 152a6ef into picklesdoc:master Aug 10, 2013
@dirkrombauts
Copy link
Member

Thanks!

@jni-
Copy link
Author

jni- commented Aug 10, 2013

You're welcome, thanks for accepting the pull request!

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.

2 participants