Add Welcome Screen
ClosedPublic

Authored by laysrodrigues on May 31 2018, 11:01 PM.

Details

Summary

Signed-off-by: Lays Rodrigues <lays.rodrigues@kde.org>

Add WelcomeWidget

  • Get RSS feed from Sith
  • TODO: Make generic for all editors
  • Basic methods for parsing the rss/xml feed

Signed-off-by: Lays Rodrigues <lays.rodrigues@kde.org>

Add WelcomeWidget to AtCoreInstanceWidget

Use WelcomeWidget as default entry point of Atelier.

Signed-off-by: Lays Rodrigues <lays.rodrigues@kde.org>

Diff Detail

Repository
R231 Atelier
Branch
welcome_screen
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 174
Build 174: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes

I had good spacing using FontMetrics().height() + 8 *5 this seams to work well across a range of dpi / font height sizes.

  • Use multiple origins for rss feed
  • Rebase
  • Fixes
rizzitello requested changes to this revision.Jun 17 2018, 3:06 AM
rizzitello added inline comments.
src/widgets/welcomewidget.cpp
97
layout->addSpacerItem(new QSpacerItem(20,20,QSizePolicy::Preferred, QSizePolicy::MinimumExpanding));

This will keep the spacing from expanding between the lines as the widget get taller.

141

I was able to get the data with in 175ms. With a wait that low the widget does it quick enough were you don't notice it was ever empty. If we are going to wait a full 2 seconds can we provide some indication here we are fetching the news?

This revision now requires changes to proceed.Jun 17 2018, 3:06 AM
laysrodrigues planned changes to this revision.Jun 18 2018, 9:29 PM
laysrodrigues marked 2 inline comments as done.
laysrodrigues added inline comments.
src/widgets/welcomewidget.cpp
141

Changed to 500ms. 200ms for me wasn't enough. I think that time will depend on the internet. Will add a fallback

laysrodrigues marked an inline comment as done.
  • Add fallback function for timeout of requests

Looking good, please clean up the commits here. we have 7 commits for this feature and some are corrections (or responses) to items here, and we all know how much tomaz like correcting commits with commits .

rizzitello requested changes to this revision.Jun 18 2018, 11:46 PM

Just noticed the first two items end with a semi colon.

This revision now requires changes to proceed.Jun 18 2018, 11:46 PM
rizzitello added inline comments.Jun 19 2018, 12:58 AM
src/widgets/welcomewidget.cpp
129

This call to fallback . Will prevent you from seeing content from a feed if a previous feed fails.
Remove it and let setuprss handle the content of the newswidget

188
"Failed to fetch the RSS Feed. \nYou may not have an internet connection.")))

the line is long why not split it? this will help keep in area if the user doesn't have it so wide they will see most of the text..

also a retry button here please.

laysrodrigues planned changes to this revision.Jun 19 2018, 1:58 AM
laysrodrigues marked an inline comment as done.
  • Add WelcomeWidget
  • Add welcome screen to mainwindow
  • Make 3DView focused when opening a file
  • Use multiple origins for rss feed

+ Sith comments

rizzitello requested changes to this revision.Jun 19 2018, 11:28 AM

I get a lot of "unable to set layout on QWidget "" because it has a layout" already if the fallback area comes up .. please check how you are setting up the newsWidget to remove these warnings.

src/widgets/welcomewidget.cpp
131

Do we want to see the fallback if one fails?

191

This button is really Small and not in line with the text.

With the refresh button I would consider making this just one line
"Failed to fetch News <retry button>"

This revision now requires changes to proceed.Jun 19 2018, 11:28 AM

I get a lot of "unable to set layout on QWidget "" because it has a layout" already if the fallback area comes up .. please check how you are setting up the newsWidget to remove these warnings.

I am working on that since last night. Will have an update on this by tomorrow.

laysrodrigues planned changes to this revision.Jun 20 2018, 2:54 AM
laysrodrigues marked 2 inline comments as done.
laysrodrigues added inline comments.
src/widgets/welcomewidget.cpp
191

idk why that came out so small. But update the phrase

laysrodrigues marked an inline comment as done.

Fixes for fallback

rizzitello requested changes to this revision.Jun 20 2018, 11:57 AM
rizzitello added inline comments.
src/widgets/welcomewidget.cpp
184

This is causing the crash

Remove all this code.

Make this new private function

void WelcomeWidget::setNewsLayout(QLayout *newLayout)
{
    if (m_newsFeedWidget->layout()) {
        qDeleteAll(m_newsFeedWidget->children());
    }
    m_newsFeedWidget->setLayout(newLayout);
}

Use that here to give the new Widget its layout
and Again in the fallback to give its layout.

no more crash no more double layouts.

200

Good Start PushButtons are ok but we should use a Tool Button here its really made for just icons. we also set the icon size to fontMetrics().lineSpacing() this should ensure out button take up all the lines height no matter the font size.

auto button = new QToolButton;
    button->setIcon(QIcon::fromTheme("view-refresh", QIcon(QString(":/%1/refresh").arg(theme))));
    button->setIconSize(QSize(fontMetrics().lineSpacing(),fontMetrics().lineSpacing()));
This revision now requires changes to proceed.Jun 20 2018, 11:57 AM
laysrodrigues planned changes to this revision.Jun 20 2018, 1:37 PM
laysrodrigues marked 2 inline comments as done.
laysrodrigues added inline comments.
src/widgets/welcomewidget.cpp
200

idk about the iconSize, because if the user gets a big font the icon will be bigger than the defaults. at least is my impression.
@tcanabrava any thoughts?

laysrodrigues marked an inline comment as done.

use QToolButton instead of QPushButton

ngraham removed a subscriber: ngraham.Jun 20 2018, 1:39 PM
rizzitello added inline comments.Jun 20 2018, 1:46 PM
src/widgets/welcomewidget.cpp
200

This is exactly what you want. This way the icon can not be to small. It will always look the right size next to the text. This keeps the button visible while giving us nice scaling.

laysrodrigues planned changes to this revision.Jun 20 2018, 1:50 PM
laysrodrigues marked an inline comment as done.

Use font size-linespacing for icons

rizzitello requested changes to this revision.Jun 20 2018, 2:03 PM
rizzitello added inline comments.
src/widgets/welcomewidget.cpp
198

missed one
setNewsLayout(layout);

This revision now requires changes to proceed.Jun 20 2018, 2:03 PM
rizzitello added inline comments.Jun 20 2018, 2:04 PM
src/widgets/welcomewidget.cpp
45

Extra empty line

198

+ empty line

laysrodrigues planned changes to this revision.Jun 20 2018, 2:05 PM
laysrodrigues marked an inline comment as done.
laysrodrigues marked 2 inline comments as done.
rizzitello added inline comments.Jun 20 2018, 2:07 PM
src/widgets/welcomewidget.cpp
119

Empty line

laysrodrigues marked an inline comment as done.Jun 20 2018, 2:08 PM
tcanabrava accepted this revision.Jun 20 2018, 2:26 PM

Well, let's merge this.
I have quite a few comments on it but they are not for now.

Remove empty lines

rizzitello accepted this revision.Jun 21 2018, 2:24 AM

Looks much better now.

patrickelectric requested changes to this revision.Jun 21 2018, 2:51 AM
patrickelectric added inline comments.
src/widgets/welcomewidget.cpp
65

Const ref

118

Ref

126

Missing this as 3rd argument of connect

127

Print error message

146

C++ for each

176

Second arg not necessary

185

Qstringliteral

src/widgets/welcomewidget.h
30

Alphabetic

42

Alphabetic order

This revision now requires changes to proceed.Jun 21 2018, 2:51 AM
This revision was not accepted when it landed; it landed in state Needs Revision.Jun 21 2018, 3:12 AM
This revision was automatically updated to reflect the committed changes.
laysrodrigues marked 3 inline comments as done.
src/widgets/welcomewidget.cpp
127

that is handled on the fallback

146

QDomNodeList doesnt implement .begin and .end.
https://doc.qt.io/qt-5/qdomnodelist.html

176

It shows better with title and date instead only the url.