openlp-core team mailing list archive

[Kirk Stover <kmstover@xxxxxxx>]

On 6/22/2014 6:50 AM, Samuel Mehrbrodt wrote:
> Review: Needs Fixing
>
> Thanks for the changes.
> Some more suggestions:
>
> * I just discovered a lot of functionality in the context menu. That's really nice. Could you put those actions in a toolbar also?
Yes - I'm not much of an artist.  Do you have an open source supplier of 
icons or is this done in-house?
> * Can you revert the changes in openlp/.version?
Yes, I don't know why that got changed
> * On my theme the theme image is shown twice per slide (vertically). Maybe you can have a look at this?
I think what you're seeing is how I handled overflows.  When the text 
exceeds the slide area,
the background is repeated (vertically) and should be ghosted with the 
custom_overflow.png to indicate an overflow condition.
> * When using "Merge with previous slide" a linebreak should be put after the first slide.
I agree
> * Is there a better name than "Tag Editor"? Maybe we can do it like Wordpress using "Text" and "Visual" for the tabs?
Sure
> * I think the visual editor should be default
I agree
> * What purpose has the custom_overflow image?
Ghosting the background on overflow - an image with transparency and opacity
> * When inserting an image:
>    * the "proportional" checkbox should be on by default.
I agree
>    * Image size: Could you allow a percentage as value? Maybe 100% should be default.
It may be tough to have pixel values and percents - any ideas?  I used 
Gimp and OpenOffice as models for my implementation.
>    * Instead of a button with the text "Select image" we usually have a button with just a folder icon (for example when selecting an image in a theme). Could you use that icon there too?
Sure
>    * The path looks like "file:////home...". There is one slash too much.
I'll check that out
> * Changing the slide color is not undoable and also seems not to work after saving.
I knew that would be confusing.  It is for display purposes only for 
when your background makes editing difficult.
For example, your theme has a transparent background, white text and the 
editor background is white.
It doesn't use an undo method, same as zoom.  Maybe it should...

It does use the theme selected from the editcustomform dialog.
>
> Ok, that's enough for now :)
> Despite these issues I'm generally surprised how well it works and how much details you took care of. Really good work!
Thanks for the review!



-- 
https://code.launchpad.net/~kirkstover/openlp/wysiwyg/+merge/223973
Your team OpenLP Core is subscribed to branch lp:openlp.