search

jon48 / webtrees-theme-rural

Rural theme for webtrees
GNU General Public License v3.0
11 stars 2 forks source link

background color male / female #24

Closed arbor95 closed 3 years ago

arbor95 commented 3 years ago

In the module vytux_cousins-master the color of the background does not differ if it is a male or female. I think it would be nice, if it looks like in the families. The classes are wt-chart-box-m and wt-chart-box-f !

jon48 commented 3 years ago

Hello, wt-chart-box-m & wt-chart-box-f are already defined as part of the theme, but it is assumed they are used in the context of a wt-chart-box element, as this is the structure defined by the core webtrees. Arguably, though, the core themes define those two classes alongside the main one, whereas the Rural theme defines as part of it.

"Externalising" wt-chart-box-m & wt-chart-box-f is a change I can probably do without too much hassle (I just need to double check it will not affect the CSS selectors specificity). I do not know the details of the Vytux Cousins module, but is there a reason it is not using the standard wt-chart-box classes structure (or even the pre-built chartbox view)? That may be better for cross-theme compatibility.

arbor95 commented 3 years ago

You can see the details at https://github.com/vytux-com/vytux_cousins .

Corresponding to your explanation I changed the two occurrences of div class="wt-chart-box--'sex'" into div class="wt-chart-box wt-chart-box--'sex'" in the tab.phtml and the male and female are colored ok.

Shall I do a pull-request at vtux-com for this change or will you change your theme corresponding to the other themes definitions of "outside"?

Testing the other themes with this "fix": That won't look good for them, because of a "fixed" height of the wt-chart-box. you can see the result at https://freris.de/tree/frese/individual/I331/Josef-Rumpf#tab-_vytux_cousins-master_

jon48 commented 3 years ago

I had a look at the module, and indeed, Vytux does not actually want to display the elements as chartboxes, he is just "hijacking" the css classes to produce the correct background color. Maybe an alternative solution would have been to use module-specific classes relying on the CSS custom properties --sex-m-bg / --sex-f-bg, but I can understand where he is coming from.

I have changed the theme to have the two classes available outside of the wt-chart-box context, in line with the core modules, so that should fix this issue.

As I had a few outstanding changes, I have issued a new release 2.0.12-v.2 including the fix alongside other improvements.