Skip to content

Conversation

@LaSi5002
Copy link
Contributor

No description provided.

@dragos-ana
Copy link
Collaborator

@LaSi5002 Thank you for your first webviewer contribution!
Suggestion: I would not remove the webviewer title -- the webviewer is part of the 4C organization, but not really the same as 4C, and this should be observable in the GUI.
Also: adding the logo via its weblink seems fine to me, but would there be any advantages / disadvantages if we added it statically, i.e. from the webviewer files?

@LaSi5002
Copy link
Contributor Author

@dragos-ana I can add the Title back in tommorrow. I just thought it was not really needed and that the UI looked better without it.
I also originally tried loading the image statically in quite a few different ways (even with a flask route). My conclusion was that because we use trame_server.core.Server and not trame.app.core.server.Server static loading is not provided by default and is quite a hassel to set up. I therefore just opted for the easy solution of loading it from github.
If you know how to (easily) statically load with trame_server.core.Server let me know. Maybe i missed something obvious.

@dragos-ana
Copy link
Collaborator

The optimal solution would be to have a 4C Webviewer logo, e.g., similar to the current 4C logo, and remove the title as you did;) Since we don't have this currently, it would make sense to still keep the title. In regards to the logo link - let's keep it as it is. And if we get a better idea on how we can add it statically, we can still change this. Thanks for the good work so far👍

@dragos-ana
Copy link
Collaborator

Very good! Please squash your commits and then I'll approve

@LaSi5002 LaSi5002 force-pushed the feature/webviewer-logo branch from acf4c38 to b09d56d Compare May 26, 2025 21:29
Copy link
Contributor

@c-p-schmidt c-p-schmidt left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@dragos-ana dragos-ana merged commit b068c65 into 4C-multiphysics:main May 28, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants