Conversation
balmas
left a comment
balmas
left a comment
There was a problem hiding this comment.
The code and responsiveness looks good to me but see my comment in vivo-project/VIVO#3972 about a problem loading the temporal google charts
| if ( groupName == "viewAll" ) { | ||
| processViewAllTab(); | ||
| } | ||
| console.log("View all") |
There was a problem hiding this comment.
suggest removing console debugging print
There was a problem hiding this comment.
Thanks, I removed it.
balmas
left a comment
balmas
left a comment
There was a problem hiding this comment.
Given that the chart issue is already fixed in main and should be resolved in the merge, I think that this looks good
I agree that Bootstrap 3 location refactor is not related to the Wilma theme directly. I can revert those changes, but would it be okay to have Bootstrap 5 on this location: (Bootstrap 3 files are stored inside js subfolder) |
With webjar there would be no need to upload bootstrap js/css assets into the project on each update and the path would /webjars/bootstrap/ (without version), which is important for customized themes update as js/css paths wouldn't change on next update. |
Added Bootstrap and updated CSS for Wilma theme to make it responsive (vivo-project#429) * Updated wilma to bootstrap 5 * Moved bootstrap to webapp folder, changed property tabs to use bootstrap * Fixed property group tabs * Bugfix: multiple dom object with same id, individual tabs * Removed duplicate bootstrap 5.3.2 * Removed unused js code in propertGroupControls * Reimplemented tab state persistence after refresh feature * Removed unnecessary code
milospp
force-pushed
the
feature/wilma-improvement
branch
from
58c6d71 to
3e89331
Compare
|
@milospp is this one ready for re-review now? |
litvinovg
left a comment
litvinovg
left a comment
There was a problem hiding this comment.
This set of PRs not only modifies Wilma theme to have responsive design, but also introduces global dependency for shared components on specific version of Bootstrap which can't safely be done retrospectively.
So it breaks backward compatibility for customized themes
https://wiki.lyrasis.org/display/VIVODOC115x/Creating+a+custom+theme
I think we should find a better way to do that.
4 participants
GitHub issue 3910
GitHub PR VIVO 3972
What does this pull request do?
Added bootstrap (only grid style) CSS and modified Wilma theme to leverage its responsive design feature and enhance the overall layout.
What's new?
Added bootstrap v5
Refactored nav tabs in Person, Org Unit page to use bootstrap component instead of jQuery
Refactored files locations
How should this be tested?
Open every page and test by resizing up to around 400px in width (Mobile device toolbar can be used in google chrome to simulate phone display)
Additional Notes:
Here is the video example
https://youtu.be/9B0_cevrEeE