Bug 239701
Description
Kevin Kofler
2007-05-10 16:02:49 UTC
Created attachment 154480 [details]
Patch to enable the userlist in KDM
Created attachment 154481 [details]
Screenshot of the result
This is a screenshot of the result taken by Sebastian Vahl.
(Note that the overlap between the labels and textboxes is due to the low
resolution, this is NOT caused by my patches.)
(In reply to comment #2) > Created an attachment (id=154481) [edit] > Screenshot of the result > > This is a screenshot of the result taken by Sebastian Vahl. > > (Note that the overlap between the labels and textboxes is due to the low > resolution, this is NOT caused by my patches.) FYI: Screenshot is taken from a vmware. Resolution there is 640x480. Kuickshow says it's actually 800×600. Created attachment 154501 [details]
Shutdown dialog with WhiteOnBlack
Uhm. The test was too quick. We really should modify WhiteOnBlack.kcsrc.
Otherwise the menus in kdm also too dark.
And I should clarify: with "modify WhiteOnBlack.kcsrc", we actually mean: make a copy of WhiteOnBlack.kcsrc called e.g. FedoraFlyingHigh.kcsrc, and then change ColorScheme=WhiteOnBlack to ColorScheme=FedoraFlyingHigh. The original WhiteOnBlack scheme should not be modified, obviously. What I wonder, however, is how much better we can get than this, because several widgets share the same palette colors. We definitely need the userlist background to be black (or, even better, #0a0a0a) for the userlist not to look like crap. Created attachment 154503 [details]
Color scheme for FedoraFlyingHigh KDM theme
So how's this one? I have:
1. started from the Plastik color scheme
2. did the minimum color changes to get lists displayed with the proper colors
(note that windowBackground is actually the "Standard background", not the
"Window background", the bad name must have stuck for backwards compatibility):
activeForeground=255,255,255
alternateBackground=10,10,10
windowBackground=10,10,10
3. replaced the 4 Plastik colors which were close to, but not quite the same as
the Fedora Blue with the real Fedora Blue (#3C6EB4, dark: #294172):
activeBackground=41,65,114
activeBlend=60,110,180
linkColor=41,65,114
selectBackground=60,110,180
This should go into /usr/share/apps/kdisplay/color-schemes/, and
ColorScheme=WhiteOnBlack changed to ColorScheme=FedoraFlyingHigh in kdmrc as
alrady discussed. I hope this looks acceptable now, it's the best I can do.
Created attachment 154504 [details]
FedoraFlyingHigh.kcsrc screenshot
This is your color-scheme. It looks ok. But another problem is, that the half
of the usernames are not readadble. I've created 6 users to see this. This also
happens with WhiteOnBlack.
Created attachment 154505 [details]
slightly modified WhiteOnBlack.kcsrc
This one is a slightly modified WhiteOnBlack. I've only changed
background=66,101,171. This is a color I've randomly token from the wallpaper.
The problem with the unreadable userlist is also present here.
Grrr, that looks like KDM is hardcoding the alternate list color instead of using the one from the color scheme. Patching that out should be a one-line fix to the code. I'm on it. Created attachment 154506 [details]
KDM patch to honor alternateBackground setting from color scheme
This is the promised code patch. It's actually 4 code lines + 3 #include
<qcolor.h> lines, not the one-liner I expected, because I have to work around a
design limitation of KApplication::createApplicationPalette: that function is
used for special apps like KDM to create a QPalette out of a KDE color scheme,
however QPalette doesn't contain KDE-specific settings, and alternateBackground
is KDE-specific. Thus, that information is lost. So, instead, I have to read it
directly from the color scheme and save it separately. (If it's missing, it
will default to QColor() which means no alternate background at all, which is a
safe default.)
This is freshly-written, so I haven't run any sorts of tests (not even checked
if it compiles) on it yet.
Created attachment 154528 [details]
KDM with FedoraFlyingHigh.kcsrc and patched kdebase
Yeah, this is much better. But has this patch any maybe negative effects to
normal color schemes?
On other explicitly-set color schemes, no, it is the right thing to do for those too! It does change the behavior on the system-wide default color scheme (i.e. if you don't set ColorScheme= at all), it disables the alternateBackground instead of just using the system-wide default in that case, I'm about to fix that. Created attachment 154539 [details]
Color scheme for FedoraFlyingHigh KDM theme - variant with 0A0A0A buttons and scrollbars
Hmmm, in that screenshot, the scrollbar could use a recoloring too... But if I
do that, that also recolors at least the buttons.
Here's another variant of FedoraFlyingHigh.kcsrc with buttonBackground and
buttonForeground changed. I don't know if this turns out better or worse than
the other variant.
Created attachment 154540 [details]
KDM patch to honor alternateBackground setting from color scheme (rev2)
And here's the fixed version of the code patch. Now if ColorScheme is not set,
the alternateBackground is just left alone as it's supposed to. This one might
even be good enough for upstream.
Created attachment 154541 [details]
Patch to enable the userlist in KDM (FedoraFlyingHigh color scheme)
And here's an updated version of the patch which started this thread, where I
changed ColorScheme (to FedoraFlyingHigh) and the associated comment.
An additional color scheme change worth investigating would be to change background to 60,110,180 (the Fedora Blue). Your blue background looked nice. If 60,110,180 doesn't look good, then we can try your 66,101,171, but I'd rather match the official Fedora colors in a FedoraFlyingHigh color scheme. ;-) Created attachment 154549 [details]
KDM with FedoraFlyingHigh2.kcsrc and patched kdebase (second patch)
Mhh. This one is FedoraFlyingHigh2.kcsrc with the new patched kdebase. It looks
like the old patch (haven't uploaded a screenshot for this). But I'm not sure
if I haven't mixed up something.
Sadly I'm not at home until sunday. So I cannot do any further tests.
Created attachment 154550 [details] FedoraFlyingHigh2.kcsrc with fedora colors and patched kdebase (second patch) And this one is with background=60,110,180 but the same issues like in comment #18 > It looks like the old patch The new code patch isn't any different from the old one if you set a ColorScheme, it only fixes the case where no ColorScheme is set (that case now works the same as without the patch, which is how it should be, the patch is only supposed to apply to the case where a ColorScheme is set explicitly). The difference between FedoraFlyingHigh.kcsrc and FedoraFlyingHigh2.kcsrc is that the scrollbar is no longer out of place (gray on black). I really like that screenshot (attachment #154550 [details]) from comment #19 (I think that's the way it should look), I'm going to update FedoraFlyingHigh.kcsrc accordingly. Created attachment 154554 [details] Color scheme for FedoraFlyingHigh KDM theme (rev3) So this one should be the good one. This is the version screenshotted in comment #19: same as FedoraFlyingHigh2.kcsrc, but with background=60,110,180. (In reply to comment #20) > > It looks like the old patch > > The new code patch isn't any different from the old one if you set a > ColorScheme, it only fixes the case where no ColorScheme is setxplicitly). Ah. Understood. (In reply to comment #21) > So this one should be the good one. This is the version screenshotted in > comment #19: same as FedoraFlyingHigh2.kcsrc, but with background=6 I agree with that. But maybe we should show this one to some people with some knowledge and experience in usuability things. Just to be sure we aren't missing something. I think we should just push it (everything: the code patch, the config patch and the color scheme, plus a default face as discussed in bug #239694) to Rawhide ASAP. If people don't like it, they'll complain anyway. And don't forget that: * Unless it gets pushed back again, the deep freeze is 6 days from now. I estimate that it will take at least one day to update the specfile, build it, wait for the build to complete, then get it pushed to f7-final. We can wait a few hours, but not days or weeks. * I can't think of anything left to improve in the color scheme (at least not without making the colors mismatch somewhere else), so I think it's no use waiting. * It's only used during login, so I don't see usability as a big concern. People used to be happy with "login:" as a login prompt. ;-) Kevin,Sebastian, it now looks great. I will build it this weekend so it should be included in FC7 release. Guys, it's great work. Thanks Ping? The weekend is over, there's still nothing in CVS, and we only have 2 days left! Kevin, the FC7 tree was locked, so i cannot build it. I have asked the rel-eng to allow building it in FC7 tree, but it's still not happen now. It's still locked. Than, building isn't locked (I've been building stuff all the past week), just tagging items for f7-final needs rel-eng approval. Uh, what tree is locked where? Core and Extras have merged now, so you should commit your work to kdebase/devel in the external CVS (the one which used to be "extras", it will work if you use the cvs.fedoraproject.org:/cvs/extras directory, though the canonical name is now cvs.fedoraproject.org:/cvs/pkgs), not in the internal Red Hat CVS. The external CVS is not locked for commits as far as I know, other packagers have been able to commit there. Once you committed the work (which hasn't happened as far as I can see from the CVS web interface), you should then run "make build" in the kdebase/devel directory, this will have the package built in Koji (which is the replacement for Brew). The package will then be tagged "dist-fc7". And then you should e-mail rel-eng and ask them to tag the package with "f7-final", that way it will go into the release. But you should get this done today or tomorrow, as the freeze happens some time on Thursday. Please see the discussions at the fedora-maintainers mailing lists if you're confused by the new processes: https://www.redhat.com/archives/fedora-maintainers/2007-May/thread.html hrm, the FedoraFlyingHigh color scheme really should be in redhat-artwork(-kde). re-assigning -> kde-settings > hrm, the FedoraFlyingHigh color scheme really should be in
> redhat-artwork(-kde).
If you can get the folks who are maintaining redhat-artwork (and now -kde too)
to include it, sure. ;-)
What's still missing in current CVS: * the color scheme (/usr/share/apps/kdisplay/color-schemes/FedoraFlyingHigh.kcsrc). If this should be in redhat-artwork-kde as Rex says, we need to bug the redhat-artwork maintainers to include it, and quickly (time is running out). * a default face: please symlink: /usr/share/apps/kdm/pics/users/default1.png (or default2 or default3) into: /usr/share/apps/kdm/faces/.default.face.icon as discussed in bug #239694. That one should definitely be done in kdebase. yes, FedoraFlyingHigh color scheme should be included in redhat-artwork-kde. i have already built redhat-artwork-kde-7.0.0-8.fc7 which includes the missing FedoraFlyingHigh.kcsrc. the kdebase-3.5.6-7.fc7 is still building. I forgot/missed that you are actually in the redhat-artwork ACL, that sure made things easier. :-) What's now in CVS should be complete, thanks. Now all that's needed is waiting for the builds to complete and then e-mailing the f7-final tagging requests for the redhat-artwork and kdebase updates to rel-eng. Keven, Sebastian. thanks for good work. when the both packages are done, i will send request to rel-eng. Rex. why have you reassigned it to kde-settings? it's a problem in kdebase. Rex, the kde-settings.spec you just imported still defaults to FedoraDNA. You'll have to change this to FedoraFlyingHigh, and then merge the configuration part of the changes from this thread into kde-settings. Please don't rush kde-settings in without checking that no configuration change from the latest kde-redhat-config-3.5-fc7-1.tar.bz2 got lost. > Rex. why have you reassigned it to kde-settings? it's a problem in kdebase.
Than, the plan (all along) was to split all configs out of kdelibs/kdebase and
put them into kde-settings. (almost done now).
Kevin, yes, I'm working to integrate that all now, but pass one certainly won't
be perfect, and will require tweaking.
You _do_ know that the deep freeze, with only fixes for blocker bugs allowed in, is coming really soon now (2 days from now is the latest date I know), right? ;-) Rex, it's too late to split all configs out of kdelibs/kdebase und put them into kde-settings. Please do it for F8 but not for F7. Bah, let's give it a shot. It'll be easy enough to back out if this turns out to be problematic. Just commited kde-settings-3.5-21, * Tue May 15 2007 Rex Dieter <rdieter[AT]fedoraproject.org> 3.5-21 - kiosk-style configs (finally) - kdm use UserLists, FedoraFlyingHigh color scheme (#239701) issued build http://koji.fedoraproject.org/koji/taskinfo?taskID=8447 redhat-artwork-7.0.0-8.fc7 and kdebase-3.5.6-7.fc7 built, now please send the rel-eng request if you didn't already. FYI: I've grabbed redhat-artwork-7.0.0-8.fc7, redhat-artwork-kde-7.0.0-8.fc7 and kdebase-3.5.6-7.fc7 from koji and spinned a new livecd with them. All is working fine here. |