Vojtech Szocs has posted comments on this change.
Change subject: userportal, webadmin: branding support.
......................................................................
Patch Set 19: (2 inline comments)
> In other words, please don't force developers to build default oVirt branding
> RPM just for sake of having default styles at runtime.
I just wanted to point out the fact that with this patch, some (default) styles
are taken away from GWT application and placed into external (oVirt default
branding package) CSS files. This means developers have to (re)install oVirt
default branding package files to given directory when:
* they want UI to look as it was before (i.e. oVirt default branding package =
new mandatory dependency for oVirt UI)
* they make changes to oVirt default branding package files and want to see
these changes in action
If you make "(re)install oVirt default branding package files to given
directory" step part of new development environment (i.e. make install-dev)
then I'm OK with that, what I'm concerned about is the conceptual change here -
taking some portion of default styles away from GWT application and into
external CSS files, which implicitly means oVirt UI will be broken unless these
files are in place.
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/branding/BrandingManager.java
Line 73: * @return A {@code List} of {@code BrandingTheme}s.
Line 74: */
Line 75: public List<BrandingTheme> getBrandingThemes() {
Line 76: if (themes == null && brandingRootPath.exists() &&
brandingRootPath.isDirectory()
Line 77: && brandingRootPath.canRead()) {
I agree with the development consideration point.
We can support theme reloading in future, for now I'm OK with current approach.
Line 78: themes = new ArrayList<BrandingTheme>();
Line 79: List<File> directories = Arrays.asList(
Line 80: brandingRootPath.listFiles(new FileFilter() {
Line 81: @Override
....................................................
File README.branding
Line 20: web_admin_css - Css to inject into web admin.
Line 21: messages - Standard java message bundle.
Line 22: version - The version of the branding in the package. Only versions
Line 23: that match the one defined in the engine will be loaded. The
current
Line 24: version is 1.
OK, for now let's keep the exact match and improve in future, if necessary.
Line 25:
Line 26: CSS INJECTION
Line 27:
Line 28: CSS are injected in the reverse order of the branding package order,
this
--
To view, visit http://gerrit.ovirt.org/13181
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a8a426ce7d688d33c5ae2b70632c836843106b2
Gerrit-PatchSet: 19
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alex Lourie <alou...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Eyal Edri <ee...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Moran Goldboim <mgold...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
