D8493: Make Folder View screen aware

[Milian Wolff]

mwolff added inline comments.

INLINE COMMENTS

> foldermodel.cpp:1671
> +        return;
> +
> +    m_screenMapper = screenMapper;

missing disconnect on the old mapper for the invalidate filter

> foldermodel.cpp:1704
> +                    setScreen(containment->screen());
> +                    connect(containment, &Plasma::Containment::screenChanged, this, &FolderModel::setScreen);
> +                }

when the interface changes, don't you need to disconnect the old containment here?

> screenmapper.cpp:58
> +
> +void ScreenMapper::addScreen(int screenId)
> +{

conceptually, what is the difference between addScreen and registerScreen? I can see the code of course, but the names are super similar, and the code as well. Can/should they share code? Most notably `m_firstScreen` isn't updated in `addScreen`?

> screenmapper.cpp:90
> +{
> +    const auto it = std::min_element(m_availableScreens.constBegin(), m_availableScreens.constEnd());
> +    return *it;

`return m_firstScreen`, no? faster, and your code would crash if there is no screen available

> screenmapper.cpp:97
> +    if (m_corona != corona) {
> +        m_corona = corona;
> +        if (m_corona) {

missing disconnect on old m_corona, if valid

REPOSITORY
  R119 Plasma Desktop

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

To: amantia, #plasma, ervin, mlaurent, dvratil, hein, aacid, davidedmundson, apol
Cc: mwolff, davidedmundson, broulik, mart, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171101/d7294919/attachment-0001.html>