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

Page colour #211

Merged
merged 10 commits into from
Jun 21, 2017
Merged

Page colour #211

merged 10 commits into from
Jun 21, 2017

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Apr 28, 2017

Allows you to change the background colour of the pages.
See https://trello.com/c/55EzQq2e/14-set-page-colour

screen shot 2017-04-29 at 23 57 49

To test:

  • Create a new figure with a few labels on the outside of the panel, at least 1 white, 1 black and 1 other color. White labels should be rendered Black (so you can see them against White page).
  • Go to File -> Page setup and pick page colour.
  • Page should update with chosen colour when Page_Setup dialog is "OK" & closed.
  • Try setting the page colour to Black.
  • Black labels should be rendered White (but not if moved to be within the panel instead of on the outside of the panel).
  • Saving and refreshing/reloading figure should preserve page colour.
  • Export of figure to PDF or TIFF should include coloured page.

NB: I'll need to upload script before the export is tested

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build FIGURE-merge#426. See the console output for more details.
Possible conflicts:

  • PR Markdown labels #209 will-moore 'Markdown labels'
    • omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py
  • PR Export to omero #210 will-moore 'Export to omero'
    • omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py

@will-moore will-moore closed this May 1, 2017
@will-moore will-moore mentioned this pull request May 3, 2017
@will-moore will-moore reopened this Jun 13, 2017
@pwalczysko
Copy link
Member

pwalczysko commented Jun 14, 2017

Bug: Export as pdf or Tiff does not preserve the feature where black labels on black bckgr are white. See screenshot, on the left is the figure, on the right the exported tiff.
screen shot 2017-06-14 at 10 21 56

@pwalczysko
Copy link
Member

All the rest works as expected, except #211 (comment). Discussed the bug with @bramalingam - if this is intended behaviour, then maybe rather remove the feature completely (feature = being clever about black on black and white on white).

@will-moore
Copy link
Member Author

To test last commit:

  • create both Black and White labels on the outside of panels
  • Choose black page - all labels should appear white and be exported as white
  • Choose white page - all labels should appear black and be exported as black
  • Choose coloured page - labels should appear and export black or white as created

@pwalczysko
Copy link
Member

Works as expected. Ready to merge.

labels[j].color == "FFFFFF") {
labels[j].color = "000000";
}
// // check that we're not adding a white label outside panel (on a white background)
Copy link
Member

Choose a reason for hiding this comment

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

remove if it is no longer needed

@@ -130,7 +130,8 @@ def __init__(self, canvas, panel, page, crop, page_height):
elif shape['type'] == "Ellipse":
self.draw_ellipse(shape)

def get_rgb(self, color):
@staticmethod
def get_rgb(color):
Copy link
Member

Choose a reason for hiding this comment

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

moving to @staticmethod means that this could be used everywhere and no need to have the same method line 424

@will-moore
Copy link
Member Author

Addressed comments from @jburel.
To test:

  • export a PDF and check that shapes (ellipse, rectangle, line & arrow) on a panel are the correct colour.

@pwalczysko
Copy link
Member

Works as expected. Ready to merge.

@jburel jburel merged commit 3273bca into ome:master Jun 21, 2017
@jburel jburel added this to the 3.1.0 milestone Jun 28, 2017
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.

4 participants