D11343: T1513

[Fabian Kristof]

fkristof added inline comments.

INLINE COMMENTS

> ferenczkovacs wrote in TextLabel.cpp:991
> I don't think so, because then i'll need a plus set of set-get functions

That's not a problem. The main idea I have here is that you use labelShape in TextLabelPrivate, and not in TextLabel, this implies that labelShape should be a member of TextLabelPrivate, not TextLabel. So I suggest you to make this change. The same applies to the other parameters you've added, like backgroundColor, borderStyle, etc, they all belong to TextLabelPrivate. TextLabel can access them through the d pointer, if needed you can write getters and setters for them.

> ferenczkovacs wrote in TextLabel.h:78
> I'll rather use const int getLabelShapeint(); 
> and delete 
> TextLabel::LabelShape getLabelShape();

Since labelShape has the type of  TextLabel::LabelShape your getter should look like

  TextLabel::LabelShape getLabelShape() const;

> ferenczkovacs wrote in LabelWidget.h:64
> a copy of the temporary shape, i think it's quite clear

I think m_previewTempLabelShape would be more self-describing in this case.

REPOSITORY
  R262 LabPlot

REVISION DETAIL
  https://phabricator.kde.org/D11343

To: ferenczkovacs, fkristof
Cc: asemke, #kde_edu, garvitkhatri, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180321/293ab204/attachment.html>