-
Notifications
You must be signed in to change notification settings - Fork 131
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
Willow Theme - New responsive variant of Wilma theme #4033
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@milospp It is hardly possible to distinguish what parts of this PR were copied from wilma theme and what are the improvements as the first commit Created the willow as copy of the wilma obviously doesn't only contain copy of wilma files, but also contain modifications related to boostrap5.
It would be much easier if the code was split into commits where it is obvious what was simply copied and what was modified and has higher chances to contain errors than copied parts.
@chenejac if it is expected to review all the styles, freemarker templates and js including the code that is part of wilma theme it could take much more time than just reviewing responsiveness modifications. I think we need clear steps of what should be reviewed and tested in this PR.
I added some comments about parts that should still be renamed from wilma to willow and a question about modifications for capability map, which created small rendering issue on other themes.
Also some menu buttons in willow theme don't work for some reason.
} | ||
|
||
#left-container, | ||
#right-container, | ||
#center-container { | ||
height:600px; | ||
/* height:600px; */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of changing capability map styles and javascript?
It created some artifacts on other themes.
Also I suggest to remove disabled styles and code (not disable it), it could be taken later from git history if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In graph.css is a lot of already commented CSS, so I lost track if that was mine or somebody else old code. I removed this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These components are utilized in every theme, so the simplest way to adapt the Willow theme responsively is to modify the root capability map CSS.
Please note that although usage of property files for translation of UI labels is supported at the moment, | ||
it is deprecated and not recommended. Please, consider using ontology instead of property file located at: | ||
Source code: [VIVO project]VIVO/home/src/main/resources/rdf/i18n/de_DE/interface-i18n/firsttime/vivo_UiLabel_de_DE_wilma.ttl | ||
Deployment: [VIVO home]/rdf/i18n/de_DE/display/interface-i18n/vivo_UiLabel_de_DE_wilma.ttl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose it should be Willow, not wilma (names and text in all txt files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed, and searched for more, and i think there are no more Wilma words inside the Willow theme
<#if !languageCount??> | ||
<#assign languageCount = 1> | ||
</#if> | ||
<#assign visRequestingTemplate = "foaf-person-wilma"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
willow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, I updated the personPublicationCountDynamicActivator like this, hope this will be good solution
Line 14 in 0bc8f43
<#if requestingTemplate == "foaf-person-wilma" || requestingTemplate == "foaf-person-willow"> |
also, the Nemo theme has the same issue
<#assign visRequestingTemplate = "foaf-person-wilma"> |
|
||
<#-- | ||
Individual profile page template for foaf:Person individuals. This is the default template for foaf persons | ||
in the Wilma theme and should reside in the themes/wilma/templates directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
willow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
I will check what happened to first commit and I will rebase so that it will be easier to compare. If you would like I can make two pull requests like this Branch A - will have only copied the Wilma theme and nothing else. I will create PR branch A <- B so that the file changed section for review contains only the changed part. after this is resolved we can create PR Main <- B If you think this approach would be easier please confirm |
To review splitting code into commits seems enough. Splitting into multiple PRs looks like an overkill. But I think it is up to @chenejac . |
GitHub issue 3910
What does this pull request do?
Created new thene based on Wilma. CSS is modified to leverage its responsive design feature and enhance the overall layout.
What's new?
Mobile optimized design
Avoided horizontal scroll
How should this be tested?
Change theme in site information and config theme to Willow
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