museeks icon indicating copy to clipboard operation
museeks copied to clipboard

Feat: Scan theme directory

Open dipras opened this issue 1 year ago • 4 comments

As requested in src/shared/lib/themes.ts to improve scaning json file instead import one by one I make the app auto scan all json in themes folder to be theme option for museeks. I used MuseeksAPI as bridge to deliver scan result to frontend because file system cannot be scanned from frontend

dipras avatar Jul 06 '23 07:07 dipras

Also, Q&A is failing, I should probably add the list of required VSCode extension required to make things work.

martpie avatar Jul 06 '23 12:07 martpie

Thanks for that! That could be a great base for custom themes, as we could imagine scanning a custom folder, like ~/.museeks/themes on startup, and users could put their themes there.

Of course that would mean we should do a couple of runtime validations, to make sure the keys are aligned with the latest theme definition, and we should fallback to _system in case a theme is invalid.

But I think we can discuss it in a later diff :)

Ah yep, that's great idea we can make an custom theme for user. We can do it later after this PR merged

dipras avatar Jul 07 '23 02:07 dipras

So I just tested it, there are two issues:

  • we do not control the order of the order of the themes anymore, which is a problem:
Screenshot 2023-07-10 at 01 55 09
  • packaged apps crash:
Screenshot 2023-07-10 at 01 53 17

As previously said, I don't think it's an important problem to be solved, but we can keep part of this code! My suggestion would be to keep the old behavior, but use this PR as a base for custom themes: could you do the same, but by scanning ~/.config/museeks/themes? Because it's an absolute path, things should not crash.

You can use this API to get the right path: https://www.electronjs.org/docs/latest/api/app#appgetpathname. appData seems the right location, but it's not easily reachable for Windows/Mac, but don't worry, I'll think about a solution. Once the solution works, we can easily change the path to a better one.

No need to do fancy checks right now, you can just steal one of the theme, change its name, update two variables and test it.

wdyt?

martpie avatar Jul 09 '23 23:07 martpie

So I just tested it, there are two issues:

  • we do not control the order of the order of the themes anymore, which is a problem:
Screenshot 2023-07-10 at 01 55 09 * packaged apps crash: Screenshot 2023-07-10 at 01 53 17 As previously said, I don't think it's an important problem to be solved, but we can keep part of this code! My suggestion would be to keep the old behavior, but use this PR as a base for custom themes: could you do the same, but by scanning `~/.config/museeks/themes`? Because it's an absolute path, things should not crash.

You can use this API to get the right path: https://www.electronjs.org/docs/latest/api/app#appgetpathname. appData seems the right location, but it's not easily reachable for Windows/Mac, but don't worry, I'll think about a solution. Once the solution works, we can easily change the path to a better one.

No need to do fancy checks right now, you can just steal one of the theme, change its name, update two variables and test it.

wdyt?

Okay, i think it's a good idea. i will try to implemant it

dipras avatar Jul 11 '23 12:07 dipras