Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added dark theme #1027

Closed
wants to merge 4 commits into from
Closed

Added dark theme #1027

wants to merge 4 commits into from

Conversation

Tyyppi77
Copy link

I added a dark theme to Tiled that can be toggled from the preference dialog. The dark theme I used is from here (https://github.com/ColinDuquesnoy/QDarkStyleSheet). This is my first time contributing, so I apologize for any problems/issues this might bring up.

@Tyyppi77
Copy link
Author

I can't make sense out of the build logs, are those errors caused by me or possibly something else? As a side note this pull request probably solves #786.

@bjorn
Copy link
Member

bjorn commented Jul 17, 2015

AppVeyor is failing since recently because some checksum verification fails while installing NSIS. That's annoying and outside the scope of our project, so I hope somebody else will fix it soon.

TravisCI is failing because it still tries to compile Tiled with Qt 4.8.1 even though the minimum supported version was raised to Qt 5.1.0.

So, don't worry about those failures.

I still need to check your changes, but thanks for doing this and I'm really curious how Tiled will look! I have a few initial requests:

  • Please amend your first commit with your second one, since there is no point in keeping the second around (you'll have to force-push to your master afterwards of course).
  • Please don't indent using tabs but use 4 spaces instead (talking about all the changes you made to Tiled's source code).
  • Please don't add the pch.h.cpp file. Its contents says that the file is auto-generated so it should not be added to the git repository. You may want to add it to .gitignore instead.

Added Dark theme

Removed unnecessary files
@Tyyppi77
Copy link
Author

I hope I got those commits right and that you don't mind the secondary commit with the tabs replaced. Otherwise I think I fulfilled your request. I'll be waiting to hear from you about the actual code changes.

@@ -508,6 +508,10 @@ MainWindow::MainWindow(QWidget *parent, Qt::WindowFlags flags)
new QShortcut(key, this, SLOT(delete_()));
#endif

Preferences *prefs = Preferences::instance();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a pointer to the preferences in this scope, called preferences.

@Tyyppi77
Copy link
Author

I hope I caught all your requests, and thanks for taking the time of describing what you wanted to be changed. How's that looking now?

@@ -120,6 +120,7 @@ QtGuiApplication {
"createscalableobjecttool.h",
"createtileobjecttool.cpp",
"createtileobjecttool.h",
"darkstyle/style.qrc",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use spaces instead of tabs for indentation.

@bjorn
Copy link
Member

bjorn commented Jul 19, 2015

I really think Tiled should have a nice dark theme. Personally I don't really like this particular one, but it's probably better than not having a dark theme (though, anybody could have already used this theme by simply passing -stylesheet darkstyle/style.qss to Tiled from the command-line).

A few problems I see with this theme:

  • There's a gray rectangle making the current zoom level unreadable.

selection_025

  • The text and background color of the groups in the Properties view don't have enough contrast compared to their surroundings.

selection_026

  • The icons on the tool bar are shifting around as you switch tools.
  • There is no hover feedback on the scroll bars.
  • Disabled icons look way too bright, but their text is unreadably dark.

selection_027

I hope we can address most these issues somehow.

@Tyyppi77
Copy link
Author

Yeah to be honest I'm not too happy with the particular theme either, but I felt like just adding something "close enough" would be fine for now... I'm not too interested in modifying the QSS file, but maybe if I notice somethings that I'd like to modify in my level-creation-workflow, I might take some time and modify the QSS where needed.

I did not know about the command-line workaround, but I wonder if that would have worked with the custom images the theme supplies?

Anyways, in my opinion having an okayish dark theme available is good enough for now, and at least there's an easy way to toggle it off if someone thinks that the problems with the theme are too annoying. But the decision of if you want this to Tiled at this state is of course yours, but as I said earlier, I don't really want to get in and modify the QSS, at least for now.

E: Sorry if I'm sounding like a dick and not wanting to do any actual work.

@bjorn
Copy link
Member

bjorn commented Jul 19, 2015

I'm not too interested in modifying the QSS file, but maybe if I notice somethings that I'd like to modify in my level-creation-workflow, I might take some time and modify the QSS where needed.

What about reporting some of these things to the original project, is that something you're interested in doing?

I did not know about the command-line workaround, but I wonder if that would have worked with the custom images the theme supplies?

I think it could work, but you're right, the way the images are referenced currently assumes the whole thing is packed as a resource file and this does not work.

But the decision of if you want this to Tiled at this state is of course yours, but as I said earlier, I don't really want to get in and modify the QSS, at least for now.

Well, I at least decided not to rush it into Tiled 0.13.

Are you maybe aware of any alternatives dark themes for Qt that we could try?

@Tyyppi77
Copy link
Author

I for sure could try to look for alternative dark themes, and give them a try. Because I can't get Tiled to build in the QtCreator, I have to dev in Visual Studio which requires me to rebuild the whole VS Solution every time I change a *.pro file (link a theme resource file), so the process is not going to be very fast. Does the fact that you decided not to rush this for 0.13 mean that there's no real rush on this? I probably could work on this when I don't feel like developing my game.

I would like to take this opportunity to thank you for all of your hard work on Tiled. The fact that I did not have to spend months building my own level editor for my games is a huge help. Tiled is awesome, but it's definitely missing a dark theme :)

@bjorn
Copy link
Member

bjorn commented Jul 20, 2015

Because I can't get Tiled to build in the QtCreator, I have to dev in Visual Studio which requires me to rebuild the whole VS Solution every time I change a *.pro file (link a theme resource file), so the process is not going to be very fast.

That sounds like a real pain. Any idea why you can't use Qt Creator?

Does the fact that you decided not to rush this for 0.13 mean that there's no real rush on this?

Yes, there's no rush at all. If it should have gone into Tiled 0.13 I would have had to merge it yesterday since I announced the string freeze for the translators.

And you're welcome for the level editor, it's been my pleasure developing it! The only reason I'm asking for donations is because otherwise I could not afford to spend much time on it, which is very frustrating.

@Tyyppi77
Copy link
Author

The Qt Creator doesn't compile *.moc files at all, which might maybe have something to do with wrong Qt version/setup. I might have to invest some time to see if I could get it to work, that would speed up stuff a lot (+ I could setup the Tiled indentation + style settings to Qt Creator and stop messing up the commits with my game-project settings which I've forgotten to change). BTW is there a style guide for Tiled somewhere?

Nice to hear that I'm not in a rush. I'll get back to this (hopefully in a couple of days, but I can't promise anything) once I've made some progress.

@bjorn
Copy link
Member

bjorn commented Jul 20, 2015

The Qt Creator doesn't compile *.moc files at all, which might maybe have something to do with wrong Qt version/setup.

Hmm yes, that sounds very strange. I haven't heard about it before. Note that personally I'm using MinGW for the daily Windows builds, so that is at least tested to work (seems the NSIS installation problem is fixed). You might have more success with that, just in case the problems are due to some unexpected VS compiler setup. In addition I'm not using the tiled.pro project for the Windows builds, but the tiled.qbs file.

BTW is there a style guide for Tiled somewhere?

It's generally the Qt coding style.

@Tyyppi77
Copy link
Author

So I ended up starting to write a theme of my own. It's quite heavily influenced by the new Hammer-editor look, and while most of the QSS is my own, I also borrowed some from the initial darkstyle I included with the pull-request. It's still WIP, and looking like this:

Tiled Dark Theme Hammer

One issue I've been unable to solve is the "Visible" checkbox in the property editor. It doesn't update to the style unless it's clicked first. I found some related code in QtBoolEdit, but it looks like it's first just a placeholder checkbox? Any pointers on this?

Some initial feedback on the theme is also welcome, but I'm really digging it and I will definitely use it for my to-come level editing.

@bjorn
Copy link
Member

bjorn commented Jul 20, 2015

@Tyyppi77 Well, I like your theme better, mainly for being less dark in general I think. Two things I don't really like is that it seems to have a lot of borders, including borders around each icon (which I think would be better to do only in hover/pressed/selected states) and even a border around the "Opacity:" label.

Personally, I would probably try to copy the style of Photoshop CS 6. I really like how it is not actually very dark, and it looks clean:

screen-shot-2012-05-12-at-4 05 24-pm

@bjorn
Copy link
Member

bjorn commented Jul 20, 2015

One issue I've been unable to solve is the "Visible" checkbox in the property editor. It doesn't update to the style unless it's clicked first. I found some related code in QtBoolEdit, but it looks like it's first just a placeholder checkbox? Any pointers on this?

This checkbox is special in that it is not really a QCheckbox widget. Rather it is just being painted as one, which might explain why it is not properly being styled by a stylesheet. Fortunately, this is also a checkbox I want to get rid of, because I'd rather have an eye icon there to make it much clearer that this is for toggling the layer visibility. Both Photoshop and GIMP also use an eye icon for this purpose.

@laetemn
Copy link
Contributor

laetemn commented Sep 30, 2015

I liked the idea of new themes ...

Which or what are the files that need to be altered to change the theme used in the Tiled?

@bjorn
Copy link
Member

bjorn commented Sep 30, 2015

@laetemn There are no files shipping with Tiled that allow you to alter its theme currently. @Tyyppi77 has done some work here on integrating an existing dark theme. So you can look at the changes in this pull request to see how that is done.

@Tyyppi77
Copy link
Author

Tyyppi77 commented Oct 1, 2015

Yeah I kinda dropped the idea, as I lost motivation after the first boost. However if someone is interested, I can share the QSS files I used for the theme in the last screenshot I posted.

@bjorn
Copy link
Member

bjorn commented Jul 3, 2016

I'll close this PR since I'm instead working on a dark theme based on the Fusion theme. See #786 for the progress on that.

@bjorn bjorn closed this Jul 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants