> On Aug. 17, 2014, 1:38 p.m., David Edmundson wrote: > > src/declarativeimports/plasmacomponents/qml/styles/TextAreaStyle.qml, line > > 30 > > <https://git.reviewboard.kde.org/r/119781/diff/2/?file=305297#file305297line30> > > > > I can see why you're doing this but I think this approach is a bit > > dangerous, it means we constantly have to match our propoerties with > > upstream's. > > > > If 5.4 introduces a new property to TextAreaStyle, we won't use the > > default implementation but instead try to use properties that doesn't exist > > which could potentially explode. > > > > What might work is: > > > > QtQuickControls.TextAreaStyle > > { > > ScrollViewStyle { > > id: svs > > } > > > > frame: svs.frame > > scrollBarBackground: svs.scrollBarBackground > > handle: svs.handle > > (and so on) > > > > textColor: theme...whatever > > > > } > > Then we don't need to redeclare the properties and have the > > cursorHandle, selectionHandle etc. > > > > There's also an abandoned upstream review request to make Scrollbars a > > single component which will solve most the duplication. We could try and > > help get that finished. > > Marco Martin wrote: > hmm, not convinced. > I see building a scrollviewstyle inside another scrollviewstyle even > worse, it creates an extra instance for basically nothing just binding > everything it implements. It's true that an addition in textareastyle in > base will give problems, but i'm not sure it's worth instancing a dead > scrollviewstyle. > > TextAreaStyle is in itself an extension of the same style's > ScrollViewStyle. base style, Desktop style and Android style all inherit from > their own scrollviews > > David Edmundson wrote: > >It's true that an addition in textareastyle in base will give problems, > but i'm not sure it's worth instancing a dead scrollviewstyle. > > Having one near empty QObject vs potentially having a shell that doesn't > load seems worth it.
ok, convinced ;) - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119781/#review64672 ----------------------------------------------------------- On Aug. 14, 2014, 11:10 a.m., Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119781/ > ----------------------------------------------------------- > > (Updated Aug. 14, 2014, 11:10 a.m.) > > > Review request for KDE Frameworks and Plasma. > > > Repository: plasma-framework > > > Description > ------- > > This ports TextArea to Qtcontrols, all old properties/functions work (except > for errorHighlight that was a stub already) > moves also the scrollview style to make everything in the same place. (needs > to be more complete still before becoming a proper import) > > > Diffs > ----- > > src/declarativeimports/plasmacomponents/qml/TextArea.qml 3f68934 > src/declarativeimports/plasmacomponents/qml/styles/ScrollViewStyle.qml > PRE-CREATION > src/declarativeimports/plasmacomponents/qml/styles/TextAreaStyle.qml > PRE-CREATION > src/declarativeimports/plasmaextracomponents/qml/ScrollArea.qml 3818142 > src/declarativeimports/plasmaextracomponents/qml/styles/ScrollViewStyle.qml > cb0b190 > > Diff: https://git.reviewboard.kde.org/r/119781/diff/ > > > Testing > ------- > > > Thanks, > > Marco Martin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
