Gerold55 / laptop

Introducing the MineTest Laptop Mod
Other
26 stars 14 forks source link

Desktop Icon Label Update #98

Closed Grizzly-Adam closed 6 years ago

bell07 commented 6 years ago

As proposed in #62, the clock color should have own prefix

Just a moment:

get_button(mtos.theme.taskbar_clock_position_and_size, "major", "os_clock", os.date("%c"))

At the time the clock uses the "major" prefix. If there is an issue with color this affects all "major" buttons? So we need "major_textcolor" instead of new prefix? The "major" is used for "Reset" button in TNTSweeper for example and for "Apply" in settings app. How it looks like?

Grizzly-Adam commented 6 years ago

That would be good. I will look at it. Just finished changes you listed from last night (some are still on my local drive as I will be making more changes).

bell07 commented 6 years ago

Do you see the discussed reorganization for theme definition https://github.com/Gerold55/minetest-laptop/issues/91#issuecomment-366895725 in this PR? If you like I can do it, and prepare the reorganization-PR to your master so my patch will be added to your PR if merged. I have time do it in ~ 5 hours

Grizzly-Adam commented 6 years ago

I will be ready for the PR in a few, if you want to wait a few minutes you can work with my files and move forward with #91

bell07 commented 6 years ago

I wait till evening (in ~5 hours) then I use your master for my changes and set up a PR with my changes to your master. Then we can discuss again if all completed or just merge to upstream if all finished

Grizzly-Adam commented 6 years ago

I am adding minor_textcolor as well because the dark themes are in need of that too.

Grizzly-Adam commented 6 years ago

OK, I am all but done. What do you think about removing the fruit theme?

bell07 commented 6 years ago

The fruit theme is by @veNext. What is wrong with the fruit theme?

Grizzly-Adam commented 6 years ago

I guess I just don't get it. It seems very odd and isn't like anything a company would send out on their computer. Maybe it's designed to look like something someone would draw in paint and set as their background?

Grizzly-Adam commented 6 years ago

Anyway, I think I have everything in place if you want to check it over.

Grizzly-Adam commented 6 years ago

Do I need to do a new PR to get everything I just changed included?

bell07 commented 6 years ago

Thank you! If you like, you can do the next changes: launcher_bg => desktop_background app_bg => app_background contrast_bg => contrast_background? (with seach+replace trough whole mod?)

Maybe some cleanup for redundancies? Inside a theme: textcolor = "#ffffff", desktop_icon_label_textcolor = "#ffffff", <= not needed because the same as textcolor in theme

Comparing to the default theme. If a theme have the same attribute as the default theme, the attribute can be skipped...

I double-check all afternoon if I am at home and can checkout the code ....

bell07 commented 6 years ago

Do I need to do a new PR to get everything I just changed included?

We are not strict with git-best-practices. The result counts. Yo do not need to set up new PR, we can continue to work on this one

Grizzly-Adam commented 6 years ago

OK, it just didn't seem like all my changes were listed here. Like the addition of the boing theme.

bell07 commented 6 years ago

No, these are different. textcolor is the color inside apps,

I meant insite the same theme because of implementation minetest.colorize(self[prefix.."_textcolor"] or self.textcolor

the "textcolor" is used as fallback for all other prefixed if no [prefix.."_textcolor"] defined. But I see you are right, if "desktop_icon_label_textcolor" is not set in a theme, the "desktop_icon_label_textcolor" value is used from default theme that is #FFFFFF, so I am right again àbout redundancy ;-)

Grizzly-Adam commented 6 years ago

I realized what you meant and deleted my response. I think we are in the same page now.

Grizzly-Adam commented 6 years ago

Too many changes due to texture names changing, deleted fork and uploading everything to new fork.

Grizzly-Adam commented 6 years ago

See PR #100

bell07 commented 6 years ago

deleted fork and uploading everything to new fork

Why? Let git manage it. I hate the "Add files via upload" commits, so I try to restore the history..

Grizzly-Adam commented 6 years ago

I thought it would make it so I didn't have to delete all those textures one by one. It didn't work. I need to figure out how to connect to github instead of using browser because it is troublesome.

I am still new to github.

bell07 commented 6 years ago

I use git command line. Which OS do you use? For Windows There is a desktop client https://desktop.github.com/ optimized for github, or git in general: tortoiseGit https://tortoisegit.org/

Grizzly-Adam commented 6 years ago

Linux mint. It's basically ubuntu.

bell07 commented 6 years ago

I am on Funtoo Linux. Tried different git-UI but none of them is really easier to understand as git command line ... Some basics:

# 1. Create a working copy
git clone https://github.com/Gerold55/minetest-laptop laptop
cd laptop

#  2. do changes

# 3. Check development / create staging
git status
git diff
git add . # . means from current directory, add all files to staging area

# 4. Check staging / create commit
git status
git diff --cached
git commit

#  5. At this point your commit is in your local repository only. You can do next changes and commits (2-4)

# 6. Move development to github
git push
Grizzly-Adam commented 6 years ago

FYI, have a look at #100 because I think it is ready to go. I even went back and changed bg refrences to background in the tnt sweeper PR after it was merged.

bell07 commented 6 years ago

I am double-checking #100 now and squash some commits. So please wait till I am done...

Grizzly-Adam commented 6 years ago

Can do. I will be making vension stew and standing by.

bell07 commented 6 years ago

Trough deletion the next files gets lost in your fork:

laptop_theme_clouds_desktop_icon_label_button.png
laptop_theme_clouds_desktop_icon_label_button_black.png
laptop_theme_clouds_desktop_icon_label_button_gray.png
laptop_theme_clouds_desktop_icon_label_button_white.png

I was able to restore them

Grizzly-Adam commented 6 years ago

Thanks. I have them locally as well.

bell07 commented 6 years ago

Try to check out your fork using git command line, then use "meld" to compare your local copy with the git one. "meld" is a graphical diff tool that you should install

bell07 commented 6 years ago

Next files not found, and I cannot restore:

laptop_theme_snowpines_minor_button.png 
laptop_theme_boing_back_button.png
laptop_theme_boing_exit_button.png
laptop_theme_boing_major_button.png

Please commit missed files

Grizzly-Adam commented 6 years ago

OK, you should have them now.