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

refactor / single file components #491

Merged
merged 12 commits into from
Dec 2, 2020

Conversation

naltatis
Copy link
Member

  • Vue Code in einzelne Single-File-Komponenten umgebaut.
  • Code-Formatierung mit Prettier und ESLint eingeführt.
  • Fontawsome Icon Importe umgestellt (besseres Treeshaking, kleineres JS Bundle)

computed: {
multi: function () {
// TODO fix compact
return this.state.loadpoints.length > 1 /* || app.compact*/;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andig Kannst du mir erklären was der Compact-Mode ist? Habe gesehen, dass das scheinbar aus der URL gezogen wird. Habs hier erstmal auskommentiert weils die globale app Variable nicht mehr gibt.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"compact" biegt die Darstellung auf den kompakteren "mehrere Ladepunkte" Modus um. Genutzt um das UI auf das 800x400 Display der OpenWB ohne scrollen zu bekommen. Das könnten wir auch später wieder einbauen.

Was ich aber nicht verstanden habe: warum das Du die app vom Singleton in eine Komponente verwandelt- ist Singleton nicht für die Root Komponente ok?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die globale app Instanzvariable hab ich entfernt weil man die eigentlich nicht braucht. Kommunikation innerhalb der App findet idR. durch Props und Events statt, nicht durch globale Referenzen. Macht das ganze auch einfacher nachvollziehbarer und testbar.

Die Toasts hätte ich auch gerne in die eigentliche App integriert, aber da das nicht ganz so trivial war hab ich's hier im PR erstmal sein gelassen und die Funktionalität über window.toasts wieder bereit gestellt.

@andig
Copy link
Member

andig commented Nov 30, 2020

Das Naming scheint noch nicht konsistent: z.B. mode vs Mode. Gibts für die Komponenten ein best practice? Tags in gross im HTML sehen für mich sehr ungewohnt aus :O

@andig andig added the enhancement New feature or request label Nov 30, 2020
<p>Minimale Beispielkonfiguration für <b>evcc</b>:</p>
<p>
<code>
<pre class="mx-3">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<pre> Format kaputt? Das könnten wir aber ggf auch ausbauen.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prettier geht bei Inhalt innerhalb von <pre> Tags auf Nummer sicher weil hier der Whitespace meaningful ist. Daher formatiert es das nicht um. Müssten wir dann händisch wieder einrücken.

@andig
Copy link
Member

andig commented Nov 30, 2020

Bei kurzem Test kein Problem. Ich würde gerne ein Release schneiden und dann weiter sehen. Diskussion auf Slack?

@naltatis
Copy link
Member Author

Das Naming scheint noch nicht konsistent: z.B. mode vs Mode. Gibts für die Komponenten ein best practice? Tags in gross im HTML sehen für mich sehr ungewohnt aus :O

Jein, eine wirkliche Best Practice gibts nicht. Auch die Doku nennt beide (Kebab und Pascal).
https://vuejs.org/v2/guide/components-registration.html#Name-Casing

Bei größeren Projekten und vor allem im Zusammenspiel mit ES-Modules hat die pascal-case Schreibweise Vorteil, dass der Komponentenname auch ein valider JS-Identifier ist. Mit Kebab-Case geht das nicht weil du dort den Bindestrich hast.

@andig andig merged commit 3d3d8b4 into evcc-io:master Dec 2, 2020
@naltatis
Copy link
Member Author

naltatis commented Dec 2, 2020

🦑

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

Successfully merging this pull request may close these issues.

2 participants