search

SolidOS / mashlib

Solid-compatible data mashup library and Data Browser
https://solidos.github.io/mashlib/dist/browse.html
MIT License
72 stars 22 forks source link

moved header and footer code out of mashlib #139

Closed timea-solid closed 2 years ago

timea-solid commented 2 years ago

Initial issue: https://github.com/solid/mashlib/issues/107 This change moves the header and footer code out of mashlib. Solid-panes is not responsible to create the main page of mashlib which in turn wires in the header and footer elements from solid-ui.

The following PRs need to be approved first (in this order):

Remark: no code was lost or deleted, it was refactored.

jeff-zucker commented 2 years ago

This feature does not appear at all in the web app (mashlib/dist/browse.html). I am strongly opposed to pushing commits to mashlib that do not work on mashlib/dist/browse.html. If they do not work there, they are unlikely to work in any non-server based version of SolidOS. If we have any commitment to use of SolidOS in other contexts, we can not see keeping the web app working as an enhancement always in the do-later pile. We don't need to necessarily test in Data Kitchen or other apps and stacks which use SolidOS, but the web app is critical.

In a very few cases there will be situations where the auth flow prevents the web app from operating like the server-based app. In those situations, we can simply note the problem and move on. A much more likely reason features would work in SolidOS but not the webapp is checking for logged-in status rather than currentUser and that should be fairly easy to fix in the code. The person who is most able to check for those kinds of issues is the person who wrote the new code.

I am absolutely willing to help debug web app issues, please do ping me with questions or problem areas. I am NOT willing to be the only member of the team who checks if the web-app works or be the one expected to fix everyone else's code in relation to the web app. We should discuss in team whether the web app is an integral part of SolidOS (I believe it is), or an appendage that we may or may not keep up to date.

jeff-zucker commented 2 years ago

The header and footer didn't work in the web app before this PR so perhaps it is not the responsibility of this PR to fix that. I'll go ahead and raise a separate issue and approve this PR.

bourgeoa commented 2 years ago

@jeff-zucker you are not the only one to test the web-app. I do it regularly either locally https://.localhost:8443/browse.html or on solid.github.io or an the test-server.

The code restructuring should have no impact on browse.html.

jeff-zucker commented 2 years ago

@bourgeoa thanks for doing that testing.

The code restructuring should have no impact on browse.html.

That is true and why I approved this PR. It seems to me that restructuring is a time when it is appropriate to look at the web app and to at least raise an issue if the feature is missing.

jeff-zucker commented 2 years ago

@bourgeoa - could we make it standard practice to make browse.html available on the server the way /mashlib.js is? Or if you don't want it available on the main server for some reason, maybe just on the test server. It would make it much easier to test things.

bourgeoa commented 2 years ago

@jeff-zucker https://solidcommunity.net:8443/browse.html or https://:8443/browse.html on the test server is running the mashlib's from the test server.

Same on the regular solidcommunity.net server.

jeff-zucker commented 2 years ago

@bourgeoa, excellent, thanks!