jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/347543 )
Change subject: Hygiene: consolidate image widening CSS and JS in applib ...................................................................... Hygiene: consolidate image widening CSS and JS in applib - Move the common CSS and JavaScript between Android and iOS from MobileApp and Android into wikimedia-page-library: https://github.com/wikimedia/wikimedia-page-library/pull/8 https://github.com/wikimedia/wikimedia-page-library/pull/17 https://github.com/wikimedia/wikipedia-ios/pull/1313 https://github.com/wikimedia/wikipedia-ios/pull/1305 - Update for CollapseTable wikimedia-page-library API change. - Use NPM default dependency order. Change-Id: I04a8621ca1eca700351fa81a4096b889fd0bf9ca Depends-On: Id0a0e04907129b3eb40c468468748eb787e838f2 Bug: T159194 --- A app/src/main/assets/applib.css M app/src/main/assets/bundle.js M app/src/main/assets/index.html M app/src/main/assets/preview.css M app/src/main/assets/styles.css M www/Gruntfile.js M www/index.html M www/js/transforms/collapseTables.js M www/js/transforms/widenImages.js M www/package.json 10 files changed, 242 insertions(+), 194 deletions(-) Approvals: jenkins-bot: Verified Mholloway: Looks good to me, approved diff --git a/app/src/main/assets/applib.css b/app/src/main/assets/applib.css new file mode 100644 index 0000000..11b4edf --- /dev/null +++ b/app/src/main/assets/applib.css @@ -0,0 +1,56 @@ +.wideImageOverride { + /* Center images. */ + display: block; + margin-left: auto; + margin-right: auto; + /* Ensure widening can take effect with proportional height change */ + height: auto !important; +} + +@media (orientation: portrait) { + .wideImageOverride { + width: 100% !important; + max-width: 100% !important; + height: auto !important; + } + + /* Tablet override */ + @media (min-device-width: 768px) { + .wideImageOverride { + width: 60% !important; + max-width: 60% !important; + height: auto !important; + } + } +} + +@media (orientation: landscape) { + .wideImageOverride { + width: 50% !important; + max-width: 50% !important; + height: auto !important; + } +} + +/* Center text beneath images now that images are centered. */ +.thumbinner, .gallerytext { + text-align: center; +} + +/* Parsoid: center text beneath images now that images are centered. */ +figcaption { + text-align: center; +} + +/* Add a little breathing room beneath thumb captions - see enwiki "Vincent van Gogh > Paris (1886-1888)" */ +.thumbcaption { + margin-bottom: 1.5em !important; +} + +/* Wrangle extra margin - see enwiki "Claude Monet > Impressionism" */ +li.gallerybox div.thumb > div { + margin: 0px !important; +} +div.gallerytext > p { + margin-top: 0px !important; +} \ No newline at end of file diff --git a/app/src/main/assets/bundle.js b/app/src/main/assets/bundle.js index 0cccf25..a6954ab 100644 --- a/app/src/main/assets/bundle.js +++ b/app/src/main/assets/bundle.js @@ -759,7 +759,7 @@ } } ); },{"../transformer":14,"../utilities":25}],17:[function(require,module,exports){ -var getTableHeader = require("applib").CollapseElement.getTableHeader; +var getTableHeader = require("applib").CollapseTable.getTableHeader; var transformer = require("../transformer"); function handleTableCollapseOrExpandClick() { @@ -1106,47 +1106,20 @@ } } ); },{"../transformer":14}],24:[function(require,module,exports){ +var maybeWidenImage = require('applib').WidenImage.maybeWidenImage; var transformer = require("../transformer"); var utilities = require("../utilities"); var maxStretchRatioAllowedBeforeRequestingHigherResolution = 1.3; -function widenAncestors (el) { - while ((el = el.parentElement) && !el.classList.contains('content_block')) { - // Only widen if there was a width setting. Keeps changes minimal. - if (el.style.width) { - el.style.width = '100%'; - } - if (el.style.maxWidth) { - el.style.maxWidth = '100%'; - } - if (el.style.float) { - el.style.float = 'none'; - } - } -} - -function shouldWidenImage(image) { - if ( - image.width >= 64 && - image.hasAttribute('srcset') && - !image.hasAttribute('hasOverflowXContainer') && - image.parentNode.className === "image" && - !utilities.isNestedInTable(image) - ) { - return true; - } else { - return false; - } -} - -function makeRoomForImageWidening(image) { - // Expand containment so css wideImageOverride width percentages can take effect. - widenAncestors (image); - - // Remove width and height attributes so wideImageOverride width percentages can take effect. - image.removeAttribute("width"); - image.removeAttribute("height"); +function isGalleryImage(image) { + return ( + image.width >= 64 && + image.hasAttribute('srcset') && + image.parentNode.className === "image" && + // todo: remove addImageOverflowContainers transform. See T160970 + !image.hasAttribute('hasOverflowXContainer') + ); } function getStretchRatio(image) { @@ -1176,17 +1149,11 @@ } } -function widenImage(image) { - makeRoomForImageWidening (image); - image.classList.add("wideImageOverride"); - useHigherResolutionImageSrcFromSrcsetIfNecessary(image); -} - -function maybeWidenImage() { +function onImageLoad() { var image = this; - image.removeEventListener('load', maybeWidenImage, false); - if (shouldWidenImage(image)) { - widenImage(image); + image.removeEventListener('load', onImageLoad, false); + if (isGalleryImage(image) && maybeWidenImage(image)) { + useHigherResolutionImageSrcFromSrcsetIfNecessary(image); } } @@ -1195,10 +1162,11 @@ for ( var i = 0; i < images.length; i++ ) { // Load event used so images w/o style or inline width/height // attributes can still have their size determined reliably. - images[i].addEventListener('load', maybeWidenImage, false); + images[i].addEventListener('load', onImageLoad, false); } } ); -},{"../transformer":14,"../utilities":25}],25:[function(require,module,exports){ + +},{"../transformer":14,"../utilities":25,"applib":26}],25:[function(require,module,exports){ function hasAncestor( el, tagName ) { if (el !== null && el.tagName === tagName) { @@ -1344,12 +1312,149 @@ return thArray; }; -var CollapseElement = { +var CollapseTable = { getTableHeader: getTableHeader }; +/** + * Returns closest ancestor of element which matches selector. + * Similar to 'closest' methods as seen here: + * https://api.jquery.com/closest/ + * https://developer.mozilla.org/en-US/docs/Web/API/Element/closest + * @param {!Element} el Element + * @param {!string} selector Selector to look for in ancestors of 'el' + * @return {?HTMLElement} Closest ancestor of 'el' matching 'selector' + */ +var findClosest = function findClosest(el, selector) { + while ((el = el.parentElement) && !el.matches(selector)) { + // Intentionally empty. + // Reminder: the parenthesis around 'el = el.parentElement' are also intentional. + } + return el; +}; + +/** + * Determines if element has a table ancestor. + * @param {!Element} el Element + * @return {boolean} Whether table ancestor of 'el' is found + */ +var isNestedInTable = function isNestedInTable(el) { + return findClosest(el, 'table') !== null; +}; + +var elementUtilities = { + findClosest: findClosest, + isNestedInTable: isNestedInTable +}; + +/** + * To widen an image element a css class called 'wideImageOverride' is applied to the image element, + * however, ancestors of the image element can prevent the widening from taking effect. This method + * makes minimal adjustments to ancestors of the image element being widened so the image widening + * can take effect. + * @param {!HTMLElement} el Element whose ancestors will be widened + */ +var widenAncestors = function widenAncestors(el) { + while ((el = el.parentElement) && !el.classList.contains('content_block')) { + // Reminder: the parenthesis around 'el = el.parentElement' are intentional. + if (el.style.width) { + el.style.width = '100%'; + } + if (el.style.maxWidth) { + el.style.maxWidth = '100%'; + } + if (el.style.float) { + el.style.float = 'none'; + } + } +}; + +/** + * Some images should not be widended. This method makes that determination. + * @param {!HTMLElement} image The image in question + * @return {boolean} Whether 'image' should be widened + */ +var shouldWidenImage = function shouldWidenImage(image) { + // Images within a "<div class='noresize'>...</div>" should not be widened. + // Example exhibiting links overlaying such an image: + // 'enwiki > Counties of England > Scope and structure > Local government' + if (elementUtilities.findClosest(image, "[class*='noresize']")) { + return false; + } + + // Side-by-side images should not be widened. Often their captions mention 'left' and 'right', so + // we don't want to widen these as doing so would stack them vertically. + // Examples exhibiting side-by-side images: + // 'enwiki > Cold Comfort (Inside No. 9) > Casting' + // 'enwiki > Vincent van Gogh > Letters' + if (elementUtilities.findClosest(image, "div[class*='tsingle']")) { + return false; + } + + // Imagemaps, which expect images to be specific sizes, should not be widened. + // Examples can be found on 'enwiki > Kingdom (biology)': + // - first non lead image is an image map + // - 'Three domains of life > Phylogenetic Tree of Life' image is an image map + if (image.hasAttribute('usemap')) { + return false; + } + + // Images in tables should not be widened - doing so can horribly mess up table layout. + if (elementUtilities.isNestedInTable(image)) { + return false; + } + + return true; +}; + +/** + * Removes barriers to images widening taking effect. + * @param {!HTMLElement} image The image in question + */ +var makeRoomForImageWidening = function makeRoomForImageWidening(image) { + widenAncestors(image); + + // Remove width and height attributes so wideImageOverride width percentages can take effect. + image.removeAttribute('width'); + image.removeAttribute('height'); +}; + +/** + * Widens the image. + * @param {!HTMLElement} image The image in question + */ +var widenImage = function widenImage(image) { + makeRoomForImageWidening(image); + image.classList.add('wideImageOverride'); +}; + +/** + * Widens an image if the image is found to be fit for widening. + * @param {!HTMLElement} image The image in question + * @return {boolean} Whether or not 'image' was widened + */ +var maybeWidenImage = function maybeWidenImage(image) { + if (shouldWidenImage(image)) { + widenImage(image); + return true; + } + return false; +}; + +var WidenImage = { + maybeWidenImage: maybeWidenImage, + test: { + shouldWidenImage: shouldWidenImage, + widenAncestors: widenAncestors + } +}; + var index = { - CollapseElement: CollapseElement + CollapseTable: CollapseTable, + WidenImage: WidenImage, + test: { + ElementUtilities: elementUtilities + } }; module.exports = index; diff --git a/app/src/main/assets/index.html b/app/src/main/assets/index.html index 95b6e14..5c45457 100644 --- a/app/src/main/assets/index.html +++ b/app/src/main/assets/index.html @@ -5,6 +5,7 @@ <base href="" /> <!-- Set dynamically when loading each page --> <script src="file:///android_asset/bundle.js"></script> <link rel="stylesheet" href="file:///android_asset/styles.css"/> + <link rel="stylesheet" href="file:///android_asset/applib.css"/> <meta name="viewport" content="width=device-width, user-scalable=no" /> </head> diff --git a/app/src/main/assets/preview.css b/app/src/main/assets/preview.css index 142879f..1e1c259 100644 --- a/app/src/main/assets/preview.css +++ b/app/src/main/assets/preview.css @@ -622,32 +622,32 @@ vertical-align: top; display: inline-block; } - + ul.gallery, li.gallerybox { zoom: 1; *display: inline; } - + ul.gallery { margin: 2px; padding: 2px; display: block; } - + li.gallerycaption { font-weight: bold; text-align: center; display: block; word-wrap: break-word; } - + li.gallerybox div.thumb { text-align: center; border: 1px solid #ccc; margin: 2px; } - + div.gallerytext { overflow: hidden; font-size: 94%; @@ -914,48 +914,6 @@ background-repeat: no-repeat; background-position: 95% 50%; background-size: 16px 16px; -} - -.wideImageOverride { - /* Center images. */ - display: block; - clear: both; - margin-left: auto; - margin-right: auto; -} -@media (orientation: portrait) { - .wideImageOverride { - width: 100% !important; - max-width: 100% !important; - height: auto !important; - } -} -@media (orientation: portrait) and (min-device-width: 768px) { - .wideImageOverride { - width: 60% !important; - max-width: 60% !important; - height: auto !important; - } -} -@media (orientation: portrait) and (min-device-width: 768px) and (-webkit-min-device-pixel-ratio: 3) { - .wideImageOverride { - width: 100% !important; - max-width: 100% !important; - height: auto !important; - } -} -@media (orientation: landscape) { - .wideImageOverride { - width: 50% !important; - max-width: 50% !important; - height: auto !important; - } -} -/* Center text beneath images now that images are centered. */ -.thumbinner, -.gallerytext, -figcaption { - text-align: center; }/* Hide the stupid editlink */ .edit-page { display: none; diff --git a/app/src/main/assets/styles.css b/app/src/main/assets/styles.css index 14a286b..97ca1cf 100644 --- a/app/src/main/assets/styles.css +++ b/app/src/main/assets/styles.css @@ -1409,48 +1409,6 @@ background-repeat: no-repeat; background-position: 95% 50%; background-size: 16px 16px; -} - -.wideImageOverride { - /* Center images. */ - display: block; - clear: both; - margin-left: auto; - margin-right: auto; -} -@media (orientation: portrait) { - .wideImageOverride { - width: 100% !important; - max-width: 100% !important; - height: auto !important; - } -} -@media (orientation: portrait) and (min-device-width: 768px) { - .wideImageOverride { - width: 60% !important; - max-width: 60% !important; - height: auto !important; - } -} -@media (orientation: portrait) and (min-device-width: 768px) and (-webkit-min-device-pixel-ratio: 3) { - .wideImageOverride { - width: 100% !important; - max-width: 100% !important; - height: auto !important; - } -} -@media (orientation: landscape) { - .wideImageOverride { - width: 50% !important; - max-width: 50% !important; - height: auto !important; - } -} -/* Center text beneath images now that images are centered. */ -.thumbinner, -.gallerytext, -figcaption { - text-align: center; }/** * Style Parsoid HTML+RDFa output consistent with wikitext from PHP parser. */ diff --git a/www/Gruntfile.js b/www/Gruntfile.js index bf6fe48..38e9a7e 100644 --- a/www/Gruntfile.js +++ b/www/Gruntfile.js @@ -92,6 +92,8 @@ // Preview files { src: [ "preview.js", "preview.html" ], dest: distFolder }, + + { src: [ "node_modules/applib/build/applib.css" ], dest: distFolder + 'applib.css' } ] } }, diff --git a/www/index.html b/www/index.html index 9d625ba..9121d91 100644 --- a/www/index.html +++ b/www/index.html @@ -5,6 +5,7 @@ <base href="" /> <!-- Set dynamically when loading each page --> <script src="file:///android_asset/bundle.js"></script> <link rel="stylesheet" href="file:///android_asset/styles.css"/> + <link rel="stylesheet" href="file:///android_asset/applib.css"/> <meta name="viewport" content="width=device-width, user-scalable=no" /> </head> diff --git a/www/js/transforms/collapseTables.js b/www/js/transforms/collapseTables.js index a44825e..f219ef5 100644 --- a/www/js/transforms/collapseTables.js +++ b/www/js/transforms/collapseTables.js @@ -1,4 +1,4 @@ -var getTableHeader = require("applib").CollapseElement.getTableHeader; +var getTableHeader = require("applib").CollapseTable.getTableHeader; var transformer = require("../transformer"); function handleTableCollapseOrExpandClick() { diff --git a/www/js/transforms/widenImages.js b/www/js/transforms/widenImages.js index 71812ac..2fa9446 100644 --- a/www/js/transforms/widenImages.js +++ b/www/js/transforms/widenImages.js @@ -1,44 +1,17 @@ +var maybeWidenImage = require('applib').WidenImage.maybeWidenImage; var transformer = require("../transformer"); var utilities = require("../utilities"); var maxStretchRatioAllowedBeforeRequestingHigherResolution = 1.3; -function widenAncestors (el) { - while ((el = el.parentElement) && !el.classList.contains('content_block')) { - // Only widen if there was a width setting. Keeps changes minimal. - if (el.style.width) { - el.style.width = '100%'; - } - if (el.style.maxWidth) { - el.style.maxWidth = '100%'; - } - if (el.style.float) { - el.style.float = 'none'; - } - } -} - -function shouldWidenImage(image) { - if ( - image.width >= 64 && - image.hasAttribute('srcset') && - !image.hasAttribute('hasOverflowXContainer') && - image.parentNode.className === "image" && - !utilities.isNestedInTable(image) - ) { - return true; - } else { - return false; - } -} - -function makeRoomForImageWidening(image) { - // Expand containment so css wideImageOverride width percentages can take effect. - widenAncestors (image); - - // Remove width and height attributes so wideImageOverride width percentages can take effect. - image.removeAttribute("width"); - image.removeAttribute("height"); +function isGalleryImage(image) { + return ( + image.width >= 64 && + image.hasAttribute('srcset') && + image.parentNode.className === "image" && + // todo: remove addImageOverflowContainers transform. See T160970 + !image.hasAttribute('hasOverflowXContainer') + ); } function getStretchRatio(image) { @@ -68,17 +41,11 @@ } } -function widenImage(image) { - makeRoomForImageWidening (image); - image.classList.add("wideImageOverride"); - useHigherResolutionImageSrcFromSrcsetIfNecessary(image); -} - -function maybeWidenImage() { +function onImageLoad() { var image = this; - image.removeEventListener('load', maybeWidenImage, false); - if (shouldWidenImage(image)) { - widenImage(image); + image.removeEventListener('load', onImageLoad, false); + if (isGalleryImage(image) && maybeWidenImage(image)) { + useHigherResolutionImageSrcFromSrcsetIfNecessary(image); } } @@ -87,6 +54,6 @@ for ( var i = 0; i < images.length; i++ ) { // Load event used so images w/o style or inline width/height // attributes can still have their size determined reliably. - images[i].addEventListener('load', maybeWidenImage, false); + images[i].addEventListener('load', onImageLoad, false); } -} ); \ No newline at end of file +} ); diff --git a/www/package.json b/www/package.json index 294733b..42c4eb8 100644 --- a/www/package.json +++ b/www/package.json @@ -4,16 +4,16 @@ "test": "grunt test" }, "dependencies": { - "applib": "0.1.2" + "applib": "1.2.2" }, "devDependencies": { - "grunt": "0.4.5", - "grunt-cli": "0.1.13", "browserify": "13.0.0", + "grunt": "0.4.5", "grunt-browserify": "4.0.1", + "grunt-cli": "0.1.13", "grunt-contrib-copy": "1.0.0", "grunt-contrib-jshint": "1.0.0", - "grunt-jsonlint": "1.0.7", - "grunt-contrib-watch": "0.6.1" + "grunt-contrib-watch": "0.6.1", + "grunt-jsonlint": "1.0.7" } } -- To view, visit https://gerrit.wikimedia.org/r/347543 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I04a8621ca1eca700351fa81a4096b889fd0bf9ca Gerrit-PatchSet: 6 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: Niedzielski <[email protected]> Gerrit-Reviewer: Brion VIBBER <[email protected]> Gerrit-Reviewer: Dbrant <[email protected]> Gerrit-Reviewer: Mholloway <[email protected]> Gerrit-Reviewer: Mhurd <[email protected]> Gerrit-Reviewer: Niedzielski <[email protected]> Gerrit-Reviewer: Sniedzielski <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
