Differential D13861

Improved Welcome Screen
AbandonedPublic
Actions

Authored by scottpetrovic on Jul 3 2018, 12:06 AM.

Details

Reviewers
woltherav
Deevad
timotheegiet
rempt
Group Reviewers
Krita
Summary

This item references this ticket...https://phabricator.kde.org/T7782

This moves a lot of stuff that was in the splash screen startup page to the blank MDI area. This also removes the "Hide on Startup" for the splash page as well as the options that are in the MDI area now. The About area still has the links.

I did add thumbnails to the recent documents. It limits the amount to 5 recenty documents.

If you hover an image over the blank area, the blank area will now have a dotted rectangle indicating the whole area is droppable.

All this code is in the petrovic/welcome-screen branch if you want to see more bite size chunks of what the changes are actually doing.

I disabled the toolbox buttons so you cannot click them any longer if no documents are open.

Test Plan

Tried creating new documents and closing documents. The correct display seems to load.

Dragging an image onto the blank area still loads the image.

I ran it by a few artists and made changes as seen on https://phabricator.kde.org/T7782

Made sure you cannot select toolbox buttons if no documents are open.

Changed themes to make sure text and everything updates

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
scottpetrovic created this revision.Jul 3 2018, 12:06 AM
Restricted Application added a project: Krita. · View Herald TranscriptJul 3 2018, 12:06 AM
scottpetrovic requested review of this revision.Jul 3 2018, 12:06 AM
dkazakov added reviewers: woltherav, Deevad, timotheegiet.Jul 3 2018, 11:45 AM
dkazakov added subscribers: Deevad, timotheegiet, woltherav, dkazakov.

Hi, @scottpetrovic!

It seems like two labels (shortcuts and the drop area) have different coloring style for some reason. It is best visible on "Blender" theme.

Other than the text color, the patch works fine for me. I think we should ask painters to test it. @woltherav, @Deevad, @timotheegiet, could you check if the patch looks fine for you?

libs/ui/KisWelcomePageWidget.cpp
167

[nitpicking mode on] I would recommend using QScopedPointer for storing this pointer [nitpicking mode off]

scottpetrovic updated this revision to Diff 37127.EditedJul 4 2018, 1:42 AM
scottpetrovic added a subscriber: rjquiralta.

thanks for the review @dkazakov !

I made the changes you requested and updated the diff.

I also talked with @rjquiralta . He indirectly spotted a small bug with the new file and open file shortcuts. It was showing an empty parenthesis if the shortcut wasn't set. I fixed that so that area is just empty if the new file or open file shortcuts are empty.

I guess we can wait to see what other artists think

scottpetrovic added a comment.Jul 4 2018, 2:13 PM

If you don't want to check out the branch and compile. I did a small video showing the changes. My audio isn't working for some reason, so check out the notes at the bottom for what I am going over (and one point I didn't)

https://youtu.be/MKRK-72tMeI

xyquadrat added a subscriber: xyquadrat.Jul 4 2018, 6:06 PM

Looks great! As discussed in T9122, it'd be good if we could place the KDE icon & something like "A KDE project" in this welcome screen. I'd either propose the area below "Drag Image in window to open" or above the "Start" title. But if you do have another idea (or don't want to add this), that is also totally fine.

scottpetrovic added a comment.EditedJul 4 2018, 6:19 PM

@xyquadrat - is just a logo going to be good enough? Would it be better to have a link in our community area that could maybe link back to somewhere in the KDE site with something like "Developed under KDE"? I am afraid that if we just put a logo, it is going to look too much like an advertisement and will get overlooked.

We do that on the homepage and I don't think it gets much attention.

I would propose it link here ( https://userbase.kde.org/What_is_KDE ) , but the first thing it says is that "so you have installed linux". 90+% of users of Krita are not on linux.

I would think it would need to go to a page that has a brief summary of what linux is...and a list of some of the applications that are built with it. Not sure what your thoughts are, or if there is a page like that

xyquadrat added a comment.Jul 5 2018, 8:22 PM

I've updated the https://userbase.kde.org/What_is_KDE page a bit, it should now be less hostile towards Windows/Mac OS users but please improve the page further if you do find any issues.

In D13861#286999, @scottpetrovic wrote:

@xyquadrat - is just a logo going to be good enough? Would it be better to have a link in our community area that could maybe link back to somewhere in the KDE site with something like "Developed under KDE"? I am afraid that if we just put a logo, it is going to look too much like an advertisement and will get overlooked.

+1, but if we simply put a link there 90% of all users will never notice it because there are many others which try to draw attention as well. So we need a solution which is
a) not too intrusive like an advertisement but
b) also getting noticed by a majority of users and
c) not a big burden to implement for Krita developers.

scottpetrovic added a comment.Jul 5 2018, 11:17 PM

@xyquadrat - Ahh. The "What is KDE" page already looks much nicer for non-technical people. Long term it might be good to have two of those pages. One for developers and all the cool stuff KDE can help with...and one like this that targets normal consumer type people that are going to be end users of the applications.

I agree that we shouldn't be distracting with the welcome page, but make it stand out when people are actually looking for that type of information. What is important is not being the loudest...but easily find-able if someone is even slightly interested. All it might take is a little KDE icon by the link in the Community section. None of the other community links have an icon, so that would really call it out. It also wouldn't be so loud that it would be seen as distracting. I am trying to think of some good text we could put.

(KDE logo) Powered by KDE
(KDE logo) Member of KDE
(KDE logo) Krita and KDE

scottpetrovic updated this revision to Diff 37213.Jul 6 2018, 12:41 AM
scottpetrovic added a subscriber: tysontan.

I have made all the updates that were requested of me.

I consulted the @tysontan and @Deevad . They liked it, and wanted an extra "clear list" button to appear by the recent documents. Clicking it would just clear out all the recent documents. The same functionality that the "Clear List" does from the File > Open Recent menu. That is coded in here too.

I dummied in the KDE link for now at the bottom for the community area. We can tweak that later if there is more discussion, but I figured that probably won't need a code review whenever we get that ironed out.

Metallicow added a subscriber: Metallicow.Jul 11 2018, 2:07 AM

This looks like a nice way to display that stuff. This is going to be an option right? I/user's might still prefer the Blender style startup dialog for example......

rempt added a subscriber: rempt.Jul 11 2018, 7:10 AM

I wouldn't make it an option. Options are expensive to maintain, hard to document and in this case, wouldn't add anything material.

rempt added a comment.Jul 11 2018, 7:18 AM

I'm building it now. I just had some nits to pick...

libs/ui/KisWelcomePageWidget.cpp
177

This will only work for ora and kra files, though -- not for psd, jpg, png or any other file format. It's tempting to use qimageio for that, but that would mean a crash on startup if the user ever opens a file that krita can load, but the qt imagio plugin would crash on.

178

Spello

179

No spaces after or before ( and )

181

I would change the logic here. First recentItem->setIcon(QIcon(QPixmap::fromImage(img)));, then check for the ora or kra thumbnail and if there's one, set it on the recentItem, instead of having two else clauses with recentItem->setIcon(QIcon(QPixmap::fromImage(img)));

182

Space after if

191

space after } -- or put else on a new line

libs/ui/KisWelcomePageWidget.h
56

member variables should be prefixed with m_

rempt added a comment.Jul 11 2018, 7:21 AM

Hm... There is something wrong with the recent documents list:

  • It doesn't show the items in the same order as file/recent
  • It doesn't show the most recent item (at least in this case)
  • It doesn't update when opening files, then closing them all so the page gets visible again.
scottpetrovic updated this revision to Diff 37752.Jul 14 2018, 3:30 PM

I did most of the updates.

  • recent file list is in the correct order
  • recent file list refreshes on welcome screen when opening and closing a document
  • fixed some typos and formatting issues

There was one comment from @rempt that I couldn't figure out what he meant. It had to do with refactoring the loading thumbnail code. He mentioned something getting used twice, but it is only being used once. I couldn't figure out how to simplify the code more without removing one of the fallbacks.

scottpetrovic marked 5 inline comments as done.Jul 14 2018, 3:33 PM
scottpetrovic added inline comments.
libs/ui/KisWelcomePageWidget.cpp
177

Are you saying to leave it for now, or is there some other way to load these formats without crashing it?

181

could you clarify a bit... I only see one reference to recentItem->setIcon(QIcon(QPixmap::fromImage(img))) so I can't condense anything with that.

rempt accepted this revision.Jul 16 2018, 10:51 AM

Looks like it works indeed :-)

libs/ui/KisWelcomePageWidget.cpp
177

Yes.

scottpetrovic abandoned this revision.Jul 17 2018, 12:12 PM

@rempt gave me the go-ahead to push this out. It is in master. closing this ticket.

Revision Contents
Changeset List

Diff 37752

View Options

libs/ui/CMakeLists.txt

Loading...
View Options

libs/ui/KisApplication.cpp

Loading...
View Options

libs/ui/KisAutoSaveRecoveryDialog.cpp

Loading...
View Options

libs/ui/KisMainWindow.h

Loading...
View Options

libs/ui/KisMainWindow.cpp

Loading...
View Options

libs/ui/KisWelcomePageWidget.h

Loading...
View Options

libs/ui/KisWelcomePageWidget.cpp

Loading...
View Options

libs/ui/dialogs/kis_dlg_preferences.h

Loading...
View Options

libs/ui/dialogs/kis_dlg_preferences.cc

Loading...
View Options

libs/ui/forms/KisWelcomePage.ui

Loading...
View Options

libs/ui/forms/wdggeneralsettings.ui

Loading...
View Options

libs/ui/kis_config.h

Loading...
View Options

libs/ui/kis_config.cc

Loading...
View Options

libs/widgetutils/config/krecentfilesaction.h

Loading...
View Options

libs/widgetutils/config/krecentfilesaction.cpp

Loading...