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

Wrong calculation of breakpoints #2917

Open
oweitman opened this issue Jan 23, 2025 · 10 comments
Open

Wrong calculation of breakpoints #2917

oweitman opened this issue Jan 23, 2025 · 10 comments
Labels

Comments

@oweitman
Copy link
Contributor

oweitman commented Jan 23, 2025

getCurrentBreakpoint(): 'xs' | 'sm' | 'md' | 'lg' | 'xl' {

https://github.com/oweitman/ioBroker.admin/blob/79bfa9e5b08535770bfdabc1fb7902de2eadde6c/packages/jsonConfig/src/JsonConfigComponent/ConfigTable.tsx#L1097

this.state.width is the width of the actual element and not the width of the screen.

the original proposed function

    iobUseMediaQuery(key: any): boolean {
        let query = this.props.theme.breakpoints.only(key);
        query = query.replace(/^@media( ?)/m, '');
        return window.matchMedia(query).matches;
    }

is taken from a hook of the next mui react version 6
https://mui.com/material-ui/react-use-media-query/
https://github.com/mui/material-ui/blob/44ba5f75ed60074892ef135fa6283a28af93a74c/packages/mui-system/src/useMediaQuery/useMediaQuery.ts

@GermanBluefox
Copy link
Contributor

Important is the width of the panel and not of the screen. We use JsonConfig in Dialogs too and in this case it is important to know the width of the dialog window.

@oweitman
Copy link
Contributor Author

oweitman commented Jan 24, 2025

for example i set a 3 column table with md = 6
then this algorithm leads to recognize the width as too small and show the card layout

if no widths are defined everything is ok,
but sometimes this breaks if the dev already defined a with

for eaxample see adapter openliga in version 1.8.0 (in v1.8.1 i enlarged the with as a workaround)
see the 1.8.0 version of jsonConfig
https://github.com/oweitman/ioBroker.openligadb/blame/81f1f11d1196c8ed65a5e69754b9d4125b1f8ed6/admin/jsonConfig.json#L23-L32

@Apollon77
Copy link
Collaborator

Hm… but if the dev defined a smaller width then this width is the relevant one for the question how to display the table and their elements inside this element. Right? Or what do I miss?

@simatec
Copy link
Contributor

simatec commented Jan 24, 2025

I think @GermanBluefox approach makes perfect sense. Problems often arise, especially with displays in dialogs. The current solution solves all problems, including tabs in popups such as in the Matter Adapter.

@oweitman
Copy link
Contributor Author

oweitman commented Jan 25, 2025

mui react defines breakpoints in terms of the screen width of a device/browser.
here the width of a jsonconfig widget is calculated and used in the same notation. this could confuse users.

as an example: the openligadb table is defined at md with 6. the screen width is md. however, the calculation from bluefox results in sx. this would mean that a user has to specify the usecardfor setting at md with sx

my original calculation and assumption is that the calculation results in the actual screen width, then compares it with the specification with usecardfor and applies it accordingly. if the dev then provides a specification for the breakpoint, you are actually flexible, as the following construct also works. screen width is md, md is 4 and usecardfor is also on md, then you are able to use the card view even with wider screens.

I don't really understand the advantages of dialogs, tabs, etc., since this discussion is only about configTable, or have I not understood something?

i add a link the the bluefox function to clarify the relation to the code

@GermanBluefox
Copy link
Contributor

Was für eine Rolle spielt die Breite von Display (Browserfenster) wenn ich meine Tabelle in einem schmalen Fenster zeige?? Ich verstehe die Logik nicht.
Browser kann 5000px sein, aber was bring es mir, wenn mein Dialogfenster 600px ist? Ich kann sowieso nicht die Tabelle mit 20 Zellen in 600px reinpuschen? In 5000px aber schon

@oweitman
Copy link
Contributor Author

oweitman commented Jan 26, 2025

Grundsätzlich so wie du es sagst ist es schon richtig.
Aber dann dürfen wir auch nicht die Breakpoint logik verwenden, da diese sich schon eindeutig auf Devicegröße bezieht.

Die ursprüngliche herangehensweise war, das Tabellenproblem für kleine/schmale Displays zu optimieren,
ohne das Nutzer da groß etwas machen müssen (zumindest wäre das für einige Adapter nicht zeitnah passiert)
Daher habe ich folgende Annahmen getroffen, so das das Table Element automatisch in die Card-Ansicht umschaltet:
Bildschirmgröße ist SX oder SM und die Tabelle hat mehr wie 3 Spalten.
Darüber hinaus noch das Attribut useCardFor mit der Angabe der Breakpoints für Devs, die dieses Feature bewusst nutzen wollen.

Durch die Berechnung der Elementbreite in deiner Funktion haben wir nun ein Bruch der Logik, da du nun die breakpoints-Notationm (wie gesagt bezieht die sich auf Gerätegreite) mit Elementbreite gleich setzt. Das könnte meiner Meinung nach die Devs verwirren. Wenn wir bei der Verwendung von Elementbreite bleiben, sollte man sich da etwas anderes überlegen.

Aber wir können es ja mal laufen lassen und sehen ob hier fragen aufkommen. Evtl vermute ich hier zu viel.
Andernfalls muss es halt durch Dokumentation ergänzt werden.

@Apollon77
Copy link
Collaborator

Am Ende können die Tabellen doch an mehreren Stellen vorkommen. Müsste die Logik dann nocht eher sein: Wie eine Tabelle dargestellt wird richtet sich nicht am screen aus, sondern an "dem container wo die Taelle drin ist". Weil so ist es dann erstmal generisch zu nutzen egal wo Jsonconfig gerade gerendert wird.

Mit Devicemanager haben wir JsconContig in Pupups bzw overlays.

Am Ende muss die Overlaygrösse bzw diese "Containergrösse" in der breite an die verfügbare screen breite angepasst werden, oder?

@oweitman
Copy link
Contributor Author

Also den Hauptanwendungszweck erfüllt die aktuelle Implementierung.
Wen ein Dev gar keine Breiten für sx,sm,... angegeben hat, dann ist die Tabelle ja meist eh so breit wie der Bildschirm
Die Berechnung (meine, wie auch die von Bluefox) merkt das und wählt dann entweder bei schmal das Kartenlayout, oder bei breit die normale Tabellendarstellung.
Es gibt nur dann ein Problem, wenn bereits eine Breite vorgegeben ist. Sow wie bei openliga. Da hatte ich für MD 6 angegeben, da die Tabelle nicht so breit ist. Aufgrund der Breitenberechnung von Bluefox, kam intern aber SX heraus und es wurde auf Kartenlayout umgeschaltet, was aber nicht sehr schön aussah.
Mittlerweile hab ich der Tabelle einfach wieder mehr Platz gegeben (MD=12, glaube ich) und das Problem ist behoben.
Wenn Devs ab jetzt in Zukunft die Einstellungen machen, können die, wenn sie es testen, auch gut einstellen.
Allerdings passt halt die nomenklatur für usecardfor nicht, da sie dann bspw MD=6 nehmen, aber in usecardfor SX nehmen müssen.
Aber das hatte ich oben bereits versucht zu beschreiben.

Aber erstmal abwarten und schauen ob diese Probleme überhaupt auftreten.

@simatec
Copy link
Contributor

simatec commented Jan 28, 2025

Ich würde aber sagen, dass fast alle Tabellen eine Breite von 12 haben.

Denke es wird maximal ne Handvoll Adapter geben, die da was extravagantes haben

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

No branches or pull requests

4 participants