search

mit-cml / appinventor-sources

MIT App Inventor Public Open Source
http://appinventor.mit.edu/appinventor-sources/
Apache License 2.0
1.48k stars 2.06k forks source link

When select dark theme, the background color of the form widget does not change to black #1299

Open wxbit opened 6 years ago

wxbit commented 6 years ago

.

ewpatton commented 6 years ago

Do you observe this phenomenon in the Companion, compiled apps, or both?

wxbit commented 6 years ago

Both in the compiled app and the companion, the background color is black when I use dark theme.

ewpatton commented 6 years ago

Ok, so then your concern is with the designer not reflecting the theme change?

wxbit commented 6 years ago

Yes, users will mistake it for a white background, It's actually black.

wxbit commented 6 years ago

Maybe should remove the dark theme?

ewpatton commented 6 years ago

The designer has never been truly WYSIWYG--there are a number of components that don't quite reflect their appearance on the device. That being said, I think this should be an easy enough fix to implement.

me9Manisha commented 5 years ago

I am looking forward to resolving this issue, can I get some direction to where in the code base this is?

ewpatton commented 5 years ago

@me9Manisha Currently the background color is set on lines 496-503 in components/src/com/google/appinventor/client/editor/simple/components/MockForm.java. Theme setting is handled starting on line 670. Note that the dark theme is controlled by adding/removing a "Dark" style to the form, but in setBackgroundColorProperty we always assume white as the default background color. What we should do instead is remove any existing background color when using the default and let the CSS do the work.

me9Manisha commented 5 years ago

After analyzing both the designer and companion sides very closely for the dark theme I came to the point that the problems in: 1.Companion side The label widget text got invisible only when text color is black but visible with other Text colors. rest is working fine in companion with a dark theme.

2.Designer side In this case, the dark theme is only reflected for buttons, not for others.

according to your suggestion, why do we need to remove the existing one as we have already a statement at line 502 deciding or setting the value for the background?

can't we follow the same concept for other widgets like buttons in the designer side for the dark theme?

ewpatton commented 5 years ago

The theory behind my suggestion builds on how CSS works. Rules are applied from least specific to most specific (and by order of CSS files, but that's irrelevant here), but styles on elements override any inferred style from CSS. So, we should really define a base style for components, then have the light/dark themes apply the relevant CSS to get that style. Then, when the developer sets a non-default property, we override the default by updating the style on the specific element. This is a more robust approach than needing every MockComponent to be aware of the theme set on the form (and in theory allows for future themes).