Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toggle DCA legends on their legend text only #1177

Merged
merged 8 commits into from
Jan 16, 2020
Merged

Toggle DCA legends on their legend text only #1177

merged 8 commits into from
Jan 16, 2020

Conversation

Tastaturberuf
Copy link
Contributor

this patch fixes the bevavior that you can't toogle the legends in DCA edits in the whitespace anymore.

all red area is toggleable
toggle_before

now the legend toggles only on the label text

BTW: i can't do the gulp things to minify the main.css

@leofeyer
Copy link
Member

This already works for me in the latest master branch. Which browser and OS are you using?

@Tastaturberuf
Copy link
Contributor Author

Tastaturberuf commented Jan 15, 2020

Mir haben uns missverstanden, mit meinem Fix kann man nicht mehr im roten Bereich toggeln. Im Whitespace toggeln ist nicht sinnvoll.
Ein ähnlicher Fix wäre auch nur per CSS möglich, ohne das zusätzliche <span>.

Wenn man dort weiter togglen können soll, wäre es schön wenn wir den Bereich einfärben.

image

@leofeyer
Copy link
Member

Aha, alles klar. Mein Fehler. 🙈

Ein ähnlicher Fix wäre auch nur per CSS möglich, ohne das zusätzliche <span>.

Das wäre mir lieber, sofern es in allen Browsern funktioniert.

@leofeyer leofeyer added the bug label Jan 16, 2020
@leofeyer leofeyer added this to the 4.9 milestone Jan 16, 2020
@Tastaturberuf
Copy link
Contributor Author

Ich mache für den CSS-Fix einen anderen PR.

@leofeyer
Copy link
Member

Warum nicht diesen nutzen? Dann hätten wir die Historie an einer Stelle.

@Tastaturberuf
Copy link
Contributor Author

Weil ich nicht weiß wie das mit dem MonoRepo funktioniert. Ich editiere im Moment nur über die Weboberfläche und will den DC_Table dann nicht nochmal anfassen.

In meiner lokalen dev-master-Installation gibt es ja nur contao/core-bundle und nicht contao/contao wo die PR hin sollen.

Vielleicht kann mich mal jemand aufklären wie ich das mit contao/contao aufsetzen kann.

@fritzmg
Copy link
Contributor

fritzmg commented Jan 16, 2020

Einfach contao/contao forken und dann in deiner composer.json requiren, statt den einzelnen bundles.

@leofeyer
Copy link
Member

Vielleicht kannst Du erstmal die CSS-Änderungen hier posten, damit wir testen können, ob diese auch in allen Browsern funktionieren.

@asaage
Copy link

asaage commented Jan 16, 2020

Im Whitespace toggeln ist nicht sinnvoll

Seh' ich persönlich anders - denn das ist in so ziemlich jedem Akkordion so.
Da würde ich mich eher dafür aussprechen, den klickbaren Bereich optisch abzuheben.

@Tastaturberuf
Copy link
Contributor Author

Welche Lösung ist mir persönlich letztlich egal, aber:
Das optisch hervorzuheben ist ne Gratwanderung zwischen relativ dunklen horizontalen Balken oder zu wenig Nutzen weil der Kontrast zu gering ist. Ohne IPS-Monitor (oder besser) und schlechter Blickwinkel und der hellgraue Background verschwindet im Pixel-Nirwana.

Noch eine Alternative wäre mit :hover wie im Foundation-CSS
Wider rum Bootstrap macht garnicht.

Danke @fritzmg. Ich probiere es gleich mal aus, dann kann ich meinen Commit ja reverten.

@Tastaturberuf
Copy link
Contributor Author

So. Schwere Geburt. Nun isses fertig. Letztlich sind es zwei Zeilen CSS.

@Tastaturberuf
Copy link
Contributor Author

BTW: Der Gulp-Task wurde nicht ausgeführt (wieder keine Ahnung wie) weswegen der Fix in der min-Version nicht drin ist.

leofeyer
leofeyer previously approved these changes Jan 16, 2020
@leofeyer leofeyer changed the title Toogle DCA legends only on label text Toggle DCA legends on their legend text only Jan 16, 2020
@leofeyer leofeyer merged commit d35c5d9 into contao:master Jan 16, 2020
@leofeyer
Copy link
Member

Thank you @Tastaturberuf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants