> 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.
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 - 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
