papayasoft/zfd2ej — comment on commit 08612805ea743e2110197f0c4660e6ec618a3b12
File:  line 0
By eddiejaoude:

Excellent, no need to create an additional branch, 'themes' branch should be enough.

I will have a play with keeping it DRY & see how it looks. Js functionality will probably be the same in all themes, and if the css is common it can always be overwritten in the theme css if required. I agree with you though, that a comfortable balance is good.

Thanks really appreciate it!

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

David Weinraub wrote:

I think I made the PR from my fork's 'themes' branch into your 'themes' branch. So, I think you can simply accept the PR, then you can add your tweaks to that branch. I don't know if you could create a separate branch to contain it, but if you want to create one, I can kill this PR and submit a new one targeted towards your desired branch. Or, I wonder if I can suggest a branch name on your side? Want me to try?

I, too, saw many places to DRY up the code, especially the duplicated stuff in multiple themes, pulling that inline js up into a common external js file. But in some sense, it is justifiable to keep it un-DRY. After all, it is the responsibility of the themer - think Wordpress or Drupal themes - to implement the specific CSS and JS that they need. Still, if the idea is that these are system-provided themes, then as long as there is something common to be shared, then we might as well do it. After all, this theming is mostly for demonstration purposes.

Anyway, I hope this helps. Cheers!

---
Reply to this email directly or view it on GitHub:
https://github.com/papayasoft/zfd2ej/commit/08612805ea743e2110197f0c4660e6ec618a3b12#commitcomment-786245