[Panel-devel] BorderLayout

Aaron J. Seigo aseigo at kde.org
Fri Sep 14 19:11:16 CEST 2007


On Friday 14 September 2007, Ivan Čukić wrote:
> It is inspired by Java's BorderLayout - there are 5 items that can be
> shown. One central and one for each border - left, right, top and bottom.

i like the idea =))

code issues:

- the Postion enums should not be ALL CAPS; and i think it makes sense to 
actually add such an enum directly to plasma.h so we don't get a bunch of 
these everywhere. currently we have Direction and (screen) Location; adding 
position would round it out nicely.

- needs a dptr =)

- can you change the licensing to gplv2 or later?

- setAllFixedSize => setFixedSizes ? though the loss of code redability for 
that one line versus 4 lines of set[Position]FixedSize is questionable. 
personally, i'd also replace those set[Position]FixedSize methods with a 
setSize(Plasma::Position pos, qreal size); or setSize(qreal size, 
Plasma::Position pos). less API and somehow feels more "natural" to me though 
that's 99% subjective =)

- why both m_items and m_itemPositions? i see that m_items is used for 
itemAt(int) and is used to reflect the order of addition, however m_items can 
easily end up with more than 5 items in it (call addItem repeatedly =) even 
though only 5 items will be shown from what i see here ... perhaps 
itemAt(int) should just index into m_itemPositions, implying a connection 
between position and ordering? would lower the overhead and avoid mess ups 
with m_items...

if m_items is kept around for whatever reason, then addItem should be changed 
to have something like this:

if (m_itemPositions[position]) {
    m_items.removeAll(m_itemPositions[position];
}

m_itemPositions[position] = item;

if (item) {
    m_items.append(item);
}

yes? =)

otherwise, i'm in favour of adding this layout to libplasma.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20070914/4c04bc0f/attachment.pgp 


More information about the Panel-devel mailing list