GNOME Bugzilla – Bug 728483
Make gnome-mines CSS stylable
Last modified: 2014-04-29 09:28:18 UTC
Currently mines uses hard-coded * drawing of each element * positioning of each element * positioning of the full table to be centered These sum up to make resizing quite slow and CPU consuming. After a discussion with aday, he missed the CSS for styling gnome-mines, and with the hard-coded drawing this is quite hard to implement. With some work, as the view is fairly well-separated from the model, the view could be changed to mostly standard GTK+ components without custom drawing. This would have the benefit of GTK+ doing the layouting, and CSS styling being available. Patches are coming up as an experiment.
Created attachment 274658 [details] [review] Part1: Move the minefield to an aspectframe Moved the minefield to an aspectframe correctly maintaining the aspect and centering of the minefield, and removed the hard-coded calculations from the minefield class.
Created attachment 274706 [details] [review] Updated patch Updated patch to move the minefield to an aspectframe after the removal of the preferences dialog.
The whole implementation (waiting for a nice CSS style) is available for testing and/or review in the wip/cssstyling branch [1], for anyone interested. Comments, suggestions are welcome. [1] https://git.gnome.org/browse/gnome-mines/log/?h=wip/cssstyling
Wow, that was fast! Allan will be happy (and I am, too). Looks like Mines will be our showcase game in 3.14, once the style gets improved. I pushed a commit to fix the path to the image files in the CSS. Bonus: now that we have a new GObject for the tiles, this also sets the stage for a quick resolution of Bug #678272.
(In reply to comment #4) > Wow, that was fast! Allan will be happy (and I am, too). Looks like Mines will > be our showcase game in 3.14, once the style gets improved. I'm still not sure how to make the neighbouring mine counts scalable, currently it just uses 18px fontsize regardless of the field size. If I can't come up with anything better (and Allan doesn't know some fancy CSS styling to do that) I think I'll just use SVGs for those too. > I pushed a commit to fix the path to the image files in the CSS. Whoops, somehow my change to do that got lost, I'm sure I've done that, along with the automake wizardry :) Anyway, thanks for the fix. > > Bonus: now that we have a new GObject for the tiles, this also sets the stage > for a quick resolution of Bug #678272.
Awesome! If you land this I'll have a play with the styling.
(In reply to comment #4) > Wow, that was fast! Allan will be happy (and I am, too). Looks like Mines will > be our showcase game in 3.14, once the style gets improved. > > I pushed a commit to fix the path to the image files in the CSS. > I'll take this as an accepted_commit-now review of the branch, and will merge into master, with high hopes to improve the CSS until 3.13.1.
Merged and pushed, ready for testing and styling on master. During styling, we must take 676612 into account, to avoid having a lagging Mines implementation, which doesn't make a good impression.
Thanks for this Robert! A few questions/comments after trying it: * What is the .cursor style for? Wasn't obvious. * It would be nice to have different classes for when the game is running and when it has been completed. You don't really want a hover class when you have completed the game, for instance. We might also want to change the colour of non-exposed tiles. * I tried adding padding between the tiles (by adding padding: 3px; to .tile) but it didn't have any effect.
(In reply to comment #9) > Thanks for this Robert! A few questions/comments after trying it: > > * What is the .cursor style for? Wasn't obvious. The cursor style is for the keyboard cursor (you can navigate the minefield using the arrow keys) > > * It would be nice to have different classes for when the game is running and > when it has been completed. You don't really want a hover class when you have > completed the game, for instance. We might also want to change the colour of > non-exposed tiles. The current style classes are ok for the running game I guess, I will add an additional class for all tiles (.completedTile) today. > > * I tried adding padding between the tiles (by adding padding: 3px; to .tile) > but it didn't have any effect. I will investigate that today to see what I can do about that.
One more thing, Allan: there have been a couple of requests to be careful about using both red and green colors on the mines, for the benefit of colorblind users.
The good news: added .explodedField and .completedField classes to all fields in game ended cases, available on master. The bad news: couldn't get the padding to work either, still don't know what the cause is, but hardcoding the spacing between the buttons is easy, so until we get a hint on how to use padding (I see that changing the global .button padding has a system-wide effect, but somehow it doesn't work for .tile ) we could set a sane value for row-spacing and column-spacing from code.
Comment on attachment 274706 [details] [review] Updated patch In master already, marking as commmitted.
Created attachment 274987 [details] Work in progress CSS file Here's my work in progress. A few things about this: * It's quite different from the mockups, and is actually intended for the light adwaita variant. * Since the tiles can be scaled, I wonder if setting a font size is appropriate in the CSS. * The main thing that's missing from the CSS support is the ability to set a color for the text and SVG images in the tiles. * There's some severe rendering issues with the SVG images (not sure if that is entirely related to the CSS or not).
(In reply to comment #14) > Created an attachment (id=274987) [details] > Work in progress CSS file > Thanks, looks better than mine, the transitions make it look nice. Some feedback though: * the 2 second transition looks a bit too much, 1 second is noticeable too, but not annoying yet for me * inspired by your CSS, adding a very subtle e.g. 0.2s transition to the .count tiles makes playing mines a lot sexier, IMHO > Here's my work in progress. A few things about this: > > * It's quite different from the mockups, and is actually intended for the > light adwaita variant. > > * Since the tiles can be scaled, I wonder if setting a font size is > appropriate in the CSS. I was hoping that you knows a trick for proportional scaling of the font size (using percentage or something) but I guess that's not possible. To properly scale the mine counts though, I think a proper solution would be to provide a numbers svg file (either one file for each number, or all numbers in one row, spaced appropriately, and then we can use background-image, background-size and background-position, that should work) > > * The main thing that's missing from the CSS support is the ability to set a > color for the text and SVG images in the tiles. Setting the color should be possible with the color attribute, at least it does work for some cases. I couldn't get it to work for insensitive buttons (and the flipped, .count tiles have been set to insensitive), so removed the insensitive setting for flipped minefields, so you can style the .count text color now. For coloring the SVG images, we either have to provide multiple coloured SVG files or do the SVG recoloring from code, that can't be done from CSS, afaik. > > * There's some severe rendering issues with the SVG images (not sure if that > is entirely related to the CSS or not). Could you provide a screenshot or something, I haven't seen anything similar on either of my PCs.
Additionally, the light blue colour for flipped tiles looks a bit out of place for me, compared to both the rest of the game and Adwaita/Gnome.
Created attachment 275008 [details] latest css file A few small changes here.
Created attachment 275009 [details] [review] patch - use the light adwaita theme The latest CSS is designed to use the light adwaita theme variant. The dark style felt a bit heavy here when I tried it.
Created attachment 275010 [details] [review] patch - artwork updates Here are the SVG images I'm using.
Very cool! We currently have a preference to display warnings when you place too many flags next to a square (e.g. two flags next to a square marked 1). That's broken right now! Maybe you could just change the color of such flags (e.g. to orange)? P.S. I'm not having any of my predicted trouble playing with the uncolorful black numbers.
Created attachment 275013 [details] Screenshot: I can select uncovered squares! I don't think I should be able to select uncovered squares -- look at the dashed inner border that appeared when I clicked on the bottom-right square in this screenshot.
Created attachment 275014 [details] Screenshot: which flag was wrong? The flag two squares to the left of the exploded mine was misplaced. It should be styled differently (after the game is over). In 3.12 we draw a big X over such a flag.
(In reply to comment #22) > Created an attachment (id=275014) [details] > Screenshot: which flag was wrong? > > The flag two squares to the left of the exploded mine was misplaced. It should > be styled differently (after the game is over). In 3.12 we draw a big X over > such a flag. I assume that is what the .incorrect CSS style is for? Maybe we need a different icon for that - flag with a cross, perhaps? (Drawing the cross over the top didn't look great to me.)
(In reply to comment #23) > (In reply to comment #22) > > Created an attachment (id=275014) [details] [details] > > Screenshot: which flag was wrong? > > > > The flag two squares to the left of the exploded mine was misplaced. It should > > be styled differently (after the game is over). In 3.12 we draw a big X over > > such a flag. > > I assume that is what the .incorrect CSS style is for? That's right, that's what the .incorrect class is for.
Thanks everyone for the feedback, it's nicely shaping up. @Michael Catanzaro: thanks for spotting the focus rectangle issue, fixed on master. @Allan Day: thanks for the updated artwork. Regarding the missing "incorrect flag" Michael mentioned, I don't feel like it MUST have an X, if we can display a wrong flag in another way, e.g. I have pushed my idea to master as a suggestion. It is a fallen flag, so still standing flags will show correctly flagged mines, and flags laying on the ground (maybe that can be drawn better than I did) could symbolize incorrectly flagged mines. Of course, that's just an idea, other ideas are welcome, if you don't like this, we can still fall back to the big X, we could draw the X as text too, and we won't have to come up with another icon (although I'm not sure which one is easier: coming up with two icons, or coming up with one icon, which looks good both with and without an X over it)
Review of attachment 275009 [details] [review]: Pushed.
Review of attachment 275010 [details] [review]: Pushed.
Created attachment 275079 [details] Screenshot of the incorrect flag idea
Created attachment 275140 [details] [review] different idea for incorrect flags The fallen flag concept doesn't really work for me - it doesn't clearly communicate what has happened. I've tried a flag icon with an X too, and that doesn't work. I think a better approach is to leave the flag in place but to change the color to red - see the attached patch.
Review of attachment 275140 [details] [review]: .explodedField .flag as you have used it, now sets the background for all flags in case of mine clicked to red, which communicates that all flags are wrong, and we still can't tell which flags are correctly put and which aren't. Did you mean .explodedField .incorrect? That works for me, incorrect flags are marked with red background, correct flags remain with their original background color.
Review of attachment 275140 [details] [review]: Committed with the fix suggested in comment 30, to make the features of the reimplemented mines on par with the features of the old implementation, including feedback on incorrectly flagged fields.
I am marking this bug as fixed, as mines is mostly stylable now, and has an initial CSS stylesheet in place. This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
Awesome! Thanks Robert. I'll put it in front of Jakub to check about the colours.