-
Notifications
You must be signed in to change notification settings - Fork 147
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
Improve Application UI #260
Conversation
Signed-off-by: yash-bharatiya_psl <yashsoc@gmail.com>
@m-reza-rahman , I have used boilerplate code for the implementation of some of the features. |
Where did the originals come from and did they have a license? |
The files src/main/webapp/resources/js/topbar.js, src/main/webapp/resources/js/topbar.min.js |
OK. MIT is a compatible license for us, so there are no issues to address. I’ll review and merge as soon as possible. |
For future reference, what are some things you keep in mind when you encounter library software or dealing with the legal aspects for the same. I have seen MIT, EPL, GPL licenses and their usages but still somewhat unclear to me. |
We are MIT ourselves, so it is straightforward. Generally any copy-left licenses are fine, but we have to evaluate compatibility on a case-by-case basis. To be honest we try to minimize copy-pasted code and external dependencies as much as possible. Even if licensing is fine, there can be other legal issues. |
Good work, @yashTEF. I tested and everything looks solid so far. |
Hi @yashTEF, as part of this work, can you please look into upgrading to PrimeFaces 12 and the "saga" theme? Our current PrimeFaces version and theme are quite old at this point. |
Indeed, while looking for possible improvements and going through the primefaces documentation I noticed that some components and themes are not available(and also the rendering and styling for the latest version is better) so this should be adopted. I have updated the dependencies but the styling and structure of JSF pages will need some changes for seamless integration. |
This PR consists of the following changes: