From 86faf05fcfdf128f08bba1657e2122a1f00cf478 Mon Sep 17 00:00:00 2001 From: Andrew Polk Date: Fri, 16 Aug 2024 14:50:49 -0700 Subject: [PATCH] Allow cover to be full-bleed image (BL-13271) --- .../localization/en/BloomMediumPriority.xlf | 12 ++ .../bookSettings/BookSettingsDialog.tsx | 46 ++++++- .../requiresBloomEnterprise.tsx | 44 +++++- src/BloomExe/Book/AppearanceSettings.cs | 125 +++++++++++++++--- src/BloomExe/Book/Book.cs | 17 ++- src/BloomExe/Book/XMatterHelper.cs | 37 +++++- src/BloomExe/Book/XMatterPackFinder.cs | 3 +- .../web/controllers/BookSettingsApi.cs | 1 - src/content/bookLayout/basePage.less | 16 ++- .../CoverIsImage-XMatter.pug | 8 ++ .../xMatter/bloom-xmatter-common.less | 10 +- .../xMatter/bloom-xmatter-mixins.pug | 2 +- 12 files changed, 287 insertions(+), 34 deletions(-) create mode 100644 src/content/templates/xMatter/CoverIsImage-XMatter/CoverIsImage-XMatter.pug diff --git a/DistFiles/localization/en/BloomMediumPriority.xlf b/DistFiles/localization/en/BloomMediumPriority.xlf index 15b3c589e238..0a5f55a73368 100644 --- a/DistFiles/localization/en/BloomMediumPriority.xlf +++ b/DistFiles/localization/en/BloomMediumPriority.xlf @@ -5,6 +5,10 @@ We are in the process of moving strings from Bloom.xlf to BloomMediumPriority.xlf and BloomLowPriority.xlf while trying very hard to preserve translations and approval status. + + Available with your Enterprise Subscription + ID: AvailableWithEnterprise + Check @@ -600,6 +604,14 @@ Cover BookSettings.CoverGroupLabel + + Fill the front cover with a single image + BookSettings.CoverIsImage + + + Using this option turns on the [Print Bleed](https://docs.bloomlibrary.org) indicators on paper layouts. See [Full Page Cover Images](https://docs.bloomlibrary.org) for information on sizing your image to fit. + BookSettings.CoverIsImage.Description + Content Pages BookSettings.ContentPagesGroupLabel diff --git a/src/BloomBrowserUI/bookEdit/bookSettings/BookSettingsDialog.tsx b/src/BloomBrowserUI/bookEdit/bookSettings/BookSettingsDialog.tsx index 6198e11b9d98..13a9b1039831 100644 --- a/src/BloomBrowserUI/bookEdit/bookSettings/BookSettingsDialog.tsx +++ b/src/BloomBrowserUI/bookEdit/bookSettings/BookSettingsDialog.tsx @@ -50,6 +50,10 @@ import { default as TrashIcon } from "@mui/icons-material/Delete"; import { PWithLink } from "../../react_components/pWithLink"; import { FieldVisibilityGroup } from "./FieldVisibilityGroup"; import { StyleAndFontTable } from "./StyleAndFontTable"; +import { + BloomEnterpriseIndicatorIconAndText, + useEnterpriseAvailable +} from "../../react_components/requiresBloomEnterprise"; let isOpenAlready = false; @@ -208,6 +212,16 @@ export const BookSettingsDialog: React.FunctionComponent<{ "BookSettings.TopLevelTextPadding.1emLabel" ); + const coverIsImageLabel = useL10n( + "Fill the front cover with a single image", + "BookSettings.CoverIsImage" + ); + //TODO real links (and change .xlf) + const coverIsImageDescription = useL10n( + "Using this option turns on the [Print Bleed](https://docs.bloomlibrary.org) indicators on paper layouts. See [Full Page Cover Images](https://docs.bloomlibrary.org) for information on sizing your image to fit.", + "BookSettings.CoverIsImage.Description" + ); + // This is a helper function to make it easier to pass the override information function getAdditionalProps( subPath: string @@ -306,6 +320,8 @@ export const BookSettingsDialog: React.FunctionComponent<{ setMigratedTheme(""); }; + const enterpriseAvailable = useEnterpriseAvailable(); + function saveSettingsAndCloseDialog() { if (settingsToReturnLater) { // If nothing changed, we don't get any...and don't need to make this call. @@ -353,7 +369,7 @@ export const BookSettingsDialog: React.FunctionComponent<{ height: 600px; // This odd width was chosen to make the customBookStyles alert box format nicely. // See BL-12956. It's not that important, but I don't think anything else is affected - // much by the exact witdh. + // much by the exact width. width: 638px; #groups { margin-right: 10px; // make room for the scrollbar @@ -398,6 +414,34 @@ export const BookSettingsDialog: React.FunctionComponent<{ label={whatToShowOnCoverLabel} path={`appearance`} > +
+ ( + `coverIsImage` + )} + disabled={ + appearanceDisabled || + !enterpriseAvailable + } + /> +
+ +
+
= props => { + const enterpriseAvailable = useEnterpriseAvailable(); + + return ( +
{ + enterpriseAvailable || + props.disabled || + openBloomEnterpriseSettings(); + }} + css={css` + display: flex; + align-items: center; + ${enterpriseAvailable || props.disabled || "cursor:pointer"}; + + opacity: ${props.disabled ? kBloomDisabledOpacity : 1.0}; + `} + className={props.className} + > + + {enterpriseAvailable ? ( + + Available with your Enterprise Subscription + + ) : ( + + Enterprise Required + + )} +
+ ); +}; + function openBloomEnterpriseSettings() { post("common/showSettingsDialog?tab=enterprise"); } diff --git a/src/BloomExe/Book/AppearanceSettings.cs b/src/BloomExe/Book/AppearanceSettings.cs index ce480fc48f56..3d4d9fe4674a 100644 --- a/src/BloomExe/Book/AppearanceSettings.cs +++ b/src/BloomExe/Book/AppearanceSettings.cs @@ -25,9 +25,9 @@ public class AppearanceSettings public AppearanceSettings() { _properties = new ExpandoObject(); - if (_substitutinator == null) + if (_appearanceMigrator == null) { - _substitutinator = new AppearanceMigrator(); + _appearanceMigrator = new AppearanceMigrator(); } // copy in the default values from each definition @@ -43,7 +43,7 @@ public AppearanceSettings() public static string kDoShowValueForDisplay = "doShow-css-will-ignore-this-and-use-default"; // by using an illegal value, we just get a no-op rule, which is what we want public static string kHideValueForDisplay = "none"; public static string kOverrideGroupsArrayKey = "groupsToOverrideFromParent"; // e.g. "coverFields, xmatter" - private static AppearanceMigrator _substitutinator; + private static AppearanceMigrator _appearanceMigrator; // A representation of the content of Appearance.json internal dynamic _properties; @@ -108,7 +108,19 @@ public string FirstPossiblyOffendingCssFile // The default here is rarely if ever relevant. Usually a newly created instance will be initialized from a folder, and the default will be overwritten, // either to whatever we find in appearance.json, or to "legacy-5-6" if there is no appearance.json. new StringPropertyDef("cssThemeName", "cssThemeName", "default"), + // BooleanPropertyDef, not CssXDef because this is not a css variable. + new BooleanPropertyDef( + "coverIsImage", + "coverIsImage", + defaultValue: false, + requiresXmatterUpdate: true + ), // If true, cover page is just a full bleed image. + // Not implemented yet. When it is, it needs to be tied together with the coverIsImage setting + // such that coverIsImage cannot be true unless fullBleed is also true. + //new BooleanPropertyDef("fullBleed", "fullBleed", false), // If true, book is full bleed. + new CssStringVariableDef("cover-background-color", "colors"), + // User can turn the visibility of these fields on and off in the dialog. new CssDisplayVariableDef("cover-title-L1-show", "coverFields", true), new CssDisplayVariableDef("cover-title-L2-show", "coverFields", true), new CssDisplayVariableDef("cover-title-L3-show", "coverFields", false), @@ -156,9 +168,62 @@ public string FirstPossiblyOffendingCssFile public string CssThemeName { get { return _properties.cssThemeName; } - set { _properties.cssThemeName = value; } + set + { + _properties.cssThemeName = value; + SetRequiredValuesIfLegacyTheme(); + } } + // Some setting values are not allowed in legacy mode. + private void SetRequiredValuesIfLegacyTheme() + { + if (CssThemeName != "legacy-5-6") // Can't use UsingLegacy here because it includes logic about the syncing of files which we don't want. + return; + + var propertyValuesRequiredByLegacyTheme = new Dictionary() + { + //REVIEW: I think that some of the other properties should also be here. + // This concept of forcing a value based on the legacy theme was introduced at the time coverIsImage was added. + // But I think the properties which existed before that and which get disabled by setting the theme + // to legacy should also be here. + // e.g. cover-topic-show + // Without this, the user can set the theme to non-legacy, change the property to whatever he wants, + // then change the theme back to legacy. + { "coverIsImage", false } + }; + foreach (var property in propertyValuesRequiredByLegacyTheme) + SetProperty(property); + } + + private void SetProperty(KeyValuePair property) + { + if ( + !Properties.ContainsKey(property.Key) + || !Properties[property.Key].Equals(property.Value) + ) + { + var propDef = propertyDefinitions.FirstOrDefault(pd => pd.Name == property.Key); + if (propDef?.RequiresXmatterUpdate == true) + PendingChangeRequiresXmatterUpdate = true; + } + + Properties[property.Key] = property.Value; + } + + public bool CoverIsImage + { + get { return _properties.coverIsImage; } + } + + //public bool FullBleed + //{ + // get { return _properties.fullBleed; } + // set { _properties.fullBleed = value; } + //} + + public bool PendingChangeRequiresXmatterUpdate; + /// /// Usually, this is simply the theme name, but if the book doesn't have one (that is, it was made by /// an earlier Bloom and has not been migrated), it answers "none". Currently this is used only @@ -682,9 +747,9 @@ public string GetCssOwnPropsDeclaration(dynamic properties, AppearanceSettings p } } - if (definition is CssPropertyDef) + if (definition is CssPropertyDef cssPropertydefinition) { - var setting = ((PropertyDef)definition).GetCssVariableDeclaration(keyValuePair); + var setting = cssPropertydefinition.GetCssVariableDeclaration(keyValuePair); if (!string.IsNullOrEmpty(setting)) cssBuilder.AppendLine("\t" + setting); } @@ -801,13 +866,14 @@ internal void UpdateFromJson(string json) { // parse the json into an object var x = JsonConvert.DeserializeObject(json); - //and then for each property, copy into the _properties object - // For backwards capabilty, if the json we are reading has a null for a value, - // do not override the default value that we already have loaded. + + // and then for each property, copy into the Properties object. + // The original comment here also said: For backwards capability, if the json we are reading has a null for a value, do not override the default value that we already have loaded. + // But the code didn't seem to implement that when I got here. (AP, Aug 2024) foreach (var property in (IDictionary)x) - { - Properties[property.Key] = property.Value; - } + SetProperty(property); + + SetRequiredValuesIfLegacyTheme(); } /// @@ -971,6 +1037,7 @@ public abstract class PropertyDef { public string Name; public dynamic DefaultValue; + public bool RequiresXmatterUpdate; public void SetDefault(dynamic prop) { @@ -981,27 +1048,47 @@ public void SetDefault(dynamic prop) /// The name of the group of properties that can a book can override from a collection, or a page can override from a book. /// public string OverrideGroup; - - public abstract string GetCssVariableDeclaration(dynamic property); } -public abstract class CssPropertyDef : PropertyDef { } - +// StringPropertyDefs and BooleanPropertyDefs get written to appearance.json but not appearance.css public class StringPropertyDef : PropertyDef { - public StringPropertyDef(string name, string overrideGroup, string defaultValue) + public StringPropertyDef( + string name, + string overrideGroup, + string defaultValue, + bool requiresXmatterUpdate = false + ) { Name = name; DefaultValue = defaultValue; OverrideGroup = overrideGroup; + RequiresXmatterUpdate = requiresXmatterUpdate; } +} - public override string GetCssVariableDeclaration(dynamic property) +public class BooleanPropertyDef : PropertyDef +{ + public BooleanPropertyDef( + string name, + string overrideGroup, + bool defaultValue, + bool requiresXmatterUpdate = false + ) { - return $"--{Name}: {property.Value};"; + Name = name; + OverrideGroup = overrideGroup; + DefaultValue = defaultValue; + RequiresXmatterUpdate = requiresXmatterUpdate; } } +// CssPropertyDefs get written to appearance.json and appearance.css +public abstract class CssPropertyDef : PropertyDef +{ + public abstract string GetCssVariableDeclaration(dynamic property); +} + public class CssStringVariableDef : CssPropertyDef { public CssStringVariableDef(string name, string overrideGroup, string defaultValue = null) diff --git a/src/BloomExe/Book/Book.cs b/src/BloomExe/Book/Book.cs index 080f908a568e..f2644c841759 100644 --- a/src/BloomExe/Book/Book.cs +++ b/src/BloomExe/Book/Book.cs @@ -2447,7 +2447,9 @@ public void BringXmatterHtmlUpToDate(HtmlDom bookDOM) layout, BookInfo.UseDeviceXMatter, _bookData.MetadataLanguage1Tag, - oldIds + oldIds, + BookInfo.AppearanceSettings.CoverIsImage + && CollectionSettings.HaveEnterpriseFeatures ); var dataBookLangs = bookDOM.GatherDataBookLanguages(); @@ -3979,8 +3981,13 @@ public void InsertFullBleedMarkup(SafeXmlElement body) } public bool FullBleed => - BookData.GetVariableOrNull("fullBleed", "*").Xml == "true" - && CollectionSettings.HaveEnterpriseFeatures; + ( + // Wants to be + // BookInfo.AppearanceSettings.FullBleed + // but we haven't put that in the book settings yet. + BookData.GetVariableOrNull("fullBleed", "*").Xml == "true" + || BookInfo.AppearanceSettings.CoverIsImage + ) && CollectionSettings.HaveEnterpriseFeatures; /// /// Save the page content to the DOM. @@ -4667,6 +4674,10 @@ public void Save() PageTemplateSource = Path.GetFileName(FolderPath); } + if (BookInfo.AppearanceSettings.PendingChangeRequiresXmatterUpdate) + EnsureUpToDateMemory(new NullProgress()); + BookInfo.AppearanceSettings.PendingChangeRequiresXmatterUpdate = false; + try { Storage.Save(); diff --git a/src/BloomExe/Book/XMatterHelper.cs b/src/BloomExe/Book/XMatterHelper.cs index 447921523a14..e6c17d532a01 100644 --- a/src/BloomExe/Book/XMatterHelper.cs +++ b/src/BloomExe/Book/XMatterHelper.cs @@ -22,6 +22,7 @@ public class XMatterHelper { private readonly HtmlDom _bookDom; private readonly string _nameOfXMatterPack; + private readonly IFileLocator _fileLocator; /// /// Constructs by finding the file and folder of the xmatter pack, given the its key name e.g. "Factory", "SILIndonesia". @@ -40,6 +41,7 @@ public XMatterHelper( bool useDeviceVersionIfAvailable = false ) { + _fileLocator = fileLocator; string directoryPath = null; _bookDom = bookDom; var bookSpecificXMatterPack = bookDom.GetMetaValue("xmatter", null); @@ -316,7 +318,8 @@ public void InjectXMatter( Layout layout, bool orderXmatterForDeviceUse, string metadataLangTag, - List oldIds = null + List oldIds = null, + bool useCoverIsImage = false ) { //don't want to pollute shells with this content @@ -358,7 +361,31 @@ SafeXmlElement xmatterPage in XMatterDom.SafeSelectNodes( ) ) { - var newPageDiv = _bookDom.RawDom.ImportNode(xmatterPage, true) as SafeXmlElement; + SafeXmlElement newPageDiv; + + // If we are using an image for the front cover, replace the typical front cover with + // a special one which has a full-page image container. + if (useCoverIsImage && IsOutsideFrontCoverPage(xmatterPage)) + { + var directoryPath = GetXMatterDirectory( + "CoverIsImage", + _fileLocator, + null, + true + ); + var coverIsImageDom = XmlHtmlConverter.GetXmlDomFromHtmlFile( + directoryPath.CombineForPath("CoverIsImage-XMatter.html"), + false + ); + var coverIsImagePage = coverIsImageDom.SelectSingleNode( + "/html/body/div[contains(@data-page,'required')]" + ); + newPageDiv = + _bookDom.RawDom.ImportNode(coverIsImagePage, true) as SafeXmlElement; + } + else + newPageDiv = _bookDom.RawDom.ImportNode(xmatterPage, true) as SafeXmlElement; + // If we're updating an existing book, we want to keep the IDs (as much as possible; sometimes // the number of xmatter pages changes and we have to add IDs). In this case, oldIds is obtained // while running RemoveExistingXMatter. @@ -502,6 +529,12 @@ public static bool IsFrontMatterPage(SafeXmlElement pageDiv) != null; } + public static bool IsOutsideFrontCoverPage(SafeXmlElement pageDiv) + { + return pageDiv.SelectSingleNode("self::div[contains(@class, 'outsideFrontCover')]") + != null; + } + public static bool IsBackMatterPage(SafeXmlElement pageDiv) { return pageDiv.SelectSingleNode("self::div[contains(@class, 'bloom-backMatter')]") diff --git a/src/BloomExe/Book/XMatterPackFinder.cs b/src/BloomExe/Book/XMatterPackFinder.cs index a4581a2aeec9..c1812d2fbb69 100644 --- a/src/BloomExe/Book/XMatterPackFinder.cs +++ b/src/BloomExe/Book/XMatterPackFinder.cs @@ -39,7 +39,8 @@ public IEnumerable All "shrp", "sharp", "forunittest", - "templatestarter" + "templatestarter", + "coverisimage", }; public IEnumerable GetXMattersToOfferInSettings( diff --git a/src/BloomExe/web/controllers/BookSettingsApi.cs b/src/BloomExe/web/controllers/BookSettingsApi.cs index c4c2cb27dff8..46f1f19137c3 100644 --- a/src/BloomExe/web/controllers/BookSettingsApi.cs +++ b/src/BloomExe/web/controllers/BookSettingsApi.cs @@ -3,7 +3,6 @@ using System.Diagnostics; using System.Dynamic; using System.IO; -using System.Linq; using Bloom.Book; using Bloom.Edit; using Bloom.web.controllers; diff --git a/src/content/bookLayout/basePage.less b/src/content/bookLayout/basePage.less index e25861423d52..ba918c3924fa 100644 --- a/src/content/bookLayout/basePage.less +++ b/src/content/bookLayout/basePage.less @@ -1,12 +1,10 @@ @import "basePage-sharedRules.less"; @import "bubble.less"; @import (reference) "../../BloomBrowserUI/bloomUI.less"; // importing by reference only brings in the constants -// NOTE: more style sheet imports are at the end of this document, -// ones that should supersede the normal rules - @import "pageBoxesSizing.less"; @import "marginBox.less"; @import "pageNumbers.less"; +// NOTE: more style sheet imports come later in this document, ones that should supersede the normal rules // TODO: replace with variable // no units! No em!!!! em makes children inside have a lineheight that matches the font of the overall box, regardless of the thing's font-size. See https://developer.mozilla.org/en-US/docs/Web/CSS/line-height @@ -14,6 +12,8 @@ // will be ignored. @defaultLineHeight: 1.5; +@dark: #222222; // duplicates @dark in common-comics.less + .Browser-Reset() { /*+init {*/ * { @@ -713,6 +713,16 @@ div.bloom-page.coverColor { overflow: hidden; } } +.bloom-page.outsideFrontCover.cover-is-image { + --cover-margin-side: 0mm; + --cover-margin-top: 0mm; + --cover-margin-bottom: 0mm; + + // This doesn't actually work currently; it only works for inside pages because the cover color overrides it. + --page-background-color: @dark !important; + + background-color: @dark !important; +} /* About the calc() here: depending on the margin, we may need to adjust the padding of the text to make it so that you get less padding on the side of the text that is next to edge of the page. diff --git a/src/content/templates/xMatter/CoverIsImage-XMatter/CoverIsImage-XMatter.pug b/src/content/templates/xMatter/CoverIsImage-XMatter/CoverIsImage-XMatter.pug new file mode 100644 index 000000000000..71d8072a5644 --- /dev/null +++ b/src/content/templates/xMatter/CoverIsImage-XMatter/CoverIsImage-XMatter.pug @@ -0,0 +1,8 @@ +include ../bloom-xmatter-mixins.pug + +doctype html +html + head + body + +page-cover('Front Cover')(data-export='front-matter-cover', data-xmatter-page='frontCover').cover-is-image.no-margin-page.frontCover.outsideFrontCover#b5169df5-6a40-4c52-bd30-4cab45afe0ed + +standard-cover-image.bloom-imageObjectFit-cover diff --git a/src/content/templates/xMatter/bloom-xmatter-common.less b/src/content/templates/xMatter/bloom-xmatter-common.less index 634f5c010cec..3e8be5b42cb0 100644 --- a/src/content/templates/xMatter/bloom-xmatter-common.less +++ b/src/content/templates/xMatter/bloom-xmatter-common.less @@ -16,7 +16,6 @@ } } - .insideFrontCover { .bloom-translationGroup { height: 100%; @@ -47,7 +46,7 @@ //independently of their appearance order in the html display: flex; flex-direction: column; - row-gap: 10px; // extra space between title items (less than default --page-gutter value which would otherwise apply) + row-gap: 10px; // extra space between title items (less than default --page-gutter value which would otherwise apply) .bloom-editable { order: 0; @@ -108,6 +107,13 @@ margin-bottom: @MarginBetweenMajorItems; } + &.cover-is-image { + .bloom-imageContainer { + margin: 0mm; + height: 100%; + } + } + // Give the image description the same order as the image we are describing, so that document order is used and we end up with it below, as it is in interior pages. (Ref BL-9946) .asideContainer { order: kFrontCoverImageOrder; diff --git a/src/content/templates/xMatter/bloom-xmatter-mixins.pug b/src/content/templates/xMatter/bloom-xmatter-mixins.pug index b8202942fb95..26cbe5087620 100644 --- a/src/content/templates/xMatter/bloom-xmatter-mixins.pug +++ b/src/content/templates/xMatter/bloom-xmatter-mixins.pug @@ -155,7 +155,7 @@ mixin standard-cover-contents //- so that the contents will survive changes to xmatter (and updates in bringBookUpToDate) mixin standard-cover-image .bloom-imageContainer - img(data-book="coverImage", src="placeHolder.png") + img(data-book="coverImage", src="placeHolder.png")&attributes(attributes) +field-cover-image("auto").bloom-imageDescription.bloom-trailingElement mixin field-cover-image(languages)