From f89b5680cb4cc28003133e0095393b78d4b87460 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 19 Jun 2023 13:24:52 -0400 Subject: [PATCH] FEATURE: Enable image grid by default (#22160) --- .../app/components/composer-editor.js | 8 +- .../instance-initializers/post-decorations.js | 28 ++-- .../javascripts/discourse/app/lib/columns.js | 10 +- .../acceptance/composer-image-grid-test.js | 1 - .../discourse/tests/unit/lib/columns-test.js | 154 ++++++++++++++++++ .../tests/unit/lib/pretty-text-test.js | 20 +-- .../engines/discourse-markdown/image-grid.js | 8 - config/locales/server.en.yml | 1 - config/site_settings.yml | 3 - 9 files changed, 176 insertions(+), 57 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/unit/lib/columns-test.js diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index c95a3e35469..1d46e8194d7 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -777,9 +777,7 @@ export default Component.extend( preview.addEventListener("click", this._handleAltTextCancelButtonClick); preview.addEventListener("click", this._handleImageDeleteButtonClick); preview.addEventListener("keypress", this._handleAltTextInputKeypress); - if (this.siteSettings.experimental_post_image_grid) { - preview.addEventListener("click", this._handleImageGridButtonClick); - } + preview.addEventListener("click", this._handleImageGridButtonClick); }, @on("willDestroyElement") @@ -806,9 +804,7 @@ export default Component.extend( preview?.removeEventListener("click", this._handleAltTextEditButtonClick); preview?.removeEventListener("click", this._handleAltTextOkButtonClick); preview?.removeEventListener("click", this._handleImageDeleteButtonClick); - if (this.siteSettings.experimental_post_image_grid) { - preview?.removeEventListener("click", this._handleImageGridButtonClick); - } + preview?.removeEventListener("click", this._handleImageGridButtonClick); preview?.removeEventListener( "click", this._handleAltTextCancelButtonClick diff --git a/app/assets/javascripts/discourse/app/instance-initializers/post-decorations.js b/app/assets/javascripts/discourse/app/instance-initializers/post-decorations.js index a43b7aa3288..c5361315a79 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/post-decorations.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/post-decorations.js @@ -33,24 +33,22 @@ export default { { id: "discourse-lightbox" } ); - if (siteSettings.experimental_post_image_grid) { - api.decorateCookedElement( - (elem) => { - const grids = elem.querySelectorAll(".d-image-grid"); + api.decorateCookedElement( + (elem) => { + const grids = elem.querySelectorAll(".d-image-grid"); - if (!grids.length) { - return; - } + if (!grids.length) { + return; + } - grids.forEach((grid) => { - return new Columns(grid, { - columns: site.mobileView ? 2 : 3, - }); + grids.forEach((grid) => { + return new Columns(grid, { + columns: site.mobileView ? 2 : 3, }); - }, - { id: "discourse-image-grid" } - ); - } + }); + }, + { id: "discourse-image-grid" } + ); if (siteSettings.support_mixed_text_direction) { api.decorateCookedElement(setTextDirections, { diff --git a/app/assets/javascripts/discourse/app/lib/columns.js b/app/assets/javascripts/discourse/app/lib/columns.js index b0ebdf106a6..3654748b987 100644 --- a/app/assets/javascripts/discourse/app/lib/columns.js +++ b/app/assets/javascripts/discourse/app/lib/columns.js @@ -4,7 +4,6 @@ * * Inspired/adapted from https://github.com/mladenilic/columns.js * - * TODO: Add unit tests */ export default class Columns { constructor(container, options = {}) { @@ -12,13 +11,10 @@ export default class Columns { this.options = { columns: 3, - columnClass: "d-image-grid-column", minCount: 2, ...options, }; - this.excluded = ["BR", "P"]; - this.items = this._prepareItems(); if (this.items.length >= this.options.minCount) { @@ -56,7 +52,7 @@ export default class Columns { const columns = []; [...Array(count)].forEach(() => { const column = document.createElement("div"); - column.classList.add(this.options.columnClass); + column.classList.add("d-image-grid-column"); columns.push(column); }); @@ -78,7 +74,7 @@ export default class Columns { }); return targets.filter((item) => { - return !this.excluded.includes(item.nodeName); + return !["BR", "P"].includes(item.nodeName); }); } @@ -111,7 +107,7 @@ export default class Columns { } // use aspect ratio to compare heights and append to shortest column - // if element is not an image, assue ratio is 1:1 + // if element is not an image, assume ratio is 1:1 const img = item.querySelector("img") || item; const aR = img.nodeName === "IMG" ? img.height / img.width : 1; columnHeights[shortest] += aR; diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-image-grid-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-image-grid-test.js index db62b7ed704..f41476d67ca 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-image-grid-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-image-grid-test.js @@ -5,7 +5,6 @@ import { test } from "qunit"; acceptance("Composer - Image Grid", function (needs) { needs.user(); needs.settings({ - experimental_post_image_grid: true, allow_uncategorized_topics: true, }); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/columns-test.js b/app/assets/javascripts/discourse/tests/unit/lib/columns-test.js new file mode 100644 index 00000000000..59fa6985675 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/unit/lib/columns-test.js @@ -0,0 +1,154 @@ +import { module, test } from "qunit"; +import Columns from "discourse/lib/columns"; + +module("Unit | Columns", function (hooks) { + hooks.afterEach(function () { + document.getElementById("qunit-fixture").innerHTML = ""; + }); + + test("works", function (assert) { + document.getElementById( + "qunit-fixture" + ).innerHTML = `
+


+
+

+
`; + + const grid = document.querySelector(".d-image-grid"); + const cols = new Columns(grid); + assert.strictEqual(cols.items.length, 3); + + assert.strictEqual( + grid.dataset.columns, + "3", + "column count attribute is correct" + ); + + assert.strictEqual( + document.querySelectorAll(".d-image-grid > .d-image-grid-column").length, + 3, + "three column elements are rendered" + ); + }); + + test("disabled if items < minCount", function (assert) { + document.getElementById( + "qunit-fixture" + ).innerHTML = `
+


+

+
`; + + const grid = document.querySelector(".d-image-grid"); + const cols = new Columns(grid, { minCount: 3 }); + + assert.strictEqual(cols.items.length, 2); + + assert.strictEqual( + grid.dataset.disabled, + "true", + "disabled attribute is added" + ); + assert.strictEqual( + document.querySelectorAll(".d-image-grid > .d-image-grid-column").length, + 0, + "no column elements are rendered" + ); + }); + + test("4 items shown in 2x2 grid", function (assert) { + document.getElementById( + "qunit-fixture" + ).innerHTML = `
+ + + + +
`; + + const grid = document.querySelector(".d-image-grid"); + const cols = new Columns(grid); + + assert.strictEqual(cols.items.length, 4); + assert.strictEqual( + grid.dataset.columns, + "2", + "column count attribute is correct" + ); + assert.strictEqual( + document.querySelectorAll(".d-image-grid > .d-image-grid-column").length, + 2, + "two columns are rendered" + ); + + assert.strictEqual( + document.querySelectorAll( + ".d-image-grid > .d-image-grid-column:first-child .image-wrapper" + ).length, + 2, + "two images in column 1" + ); + + assert.strictEqual( + document.querySelectorAll( + ".d-image-grid > .d-image-grid-column:nth-child(2) .image-wrapper" + ).length, + 2, + "two images in column 2" + ); + }); + + test("non-image elements", function (assert) { + document.getElementById( + "qunit-fixture" + ).innerHTML = `
+ + + +
hey there
+
hey there
+
`; + + const grid = document.querySelector(".d-image-grid"); + const cols = new Columns(grid); + + assert.strictEqual(cols.items.length, 5); + assert.strictEqual(cols.container, grid); + + assert.strictEqual( + grid.dataset.columns, + "3", + "column count attribute is correct" + ); + assert.strictEqual( + document.querySelectorAll(".d-image-grid > .d-image-grid-column").length, + 3, + "three columns are rendered" + ); + + assert.strictEqual( + document.querySelectorAll( + ".d-image-grid > .d-image-grid-column:first-child > *" + ).length, + 2, + "two elements in column 1" + ); + + assert.strictEqual( + document.querySelectorAll( + ".d-image-grid > .d-image-grid-column:nth-child(2) > *" + ).length, + 2, + "two elements in column 2" + ); + + assert.strictEqual( + document.querySelectorAll( + ".d-image-grid > .d-image-grid-column:nth-child(3) > *" + ).length, + 1, + "one element in column 3" + ); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js index fe3b06b8be5..c909f43b9ac 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js @@ -1754,28 +1754,18 @@ var bar = 'bar'; test("image grid", function (assert) { assert.cooked( "[grid]\n![](http://folksy.com/images/folksy-colour.png)\n[/grid]", - `

[grid]
-
-[/grid]

`, - "image grid without site setting does not work" - ); - - assert.cookedOptions( - "[grid]\n![](http://folksy.com/images/folksy-colour.png)\n[/grid]", - { siteSettings: { experimental_post_image_grid: true } }, `

`, - "image grid with site setting works" + "image grid works" ); - assert.cookedOptions( + assert.cooked( `[grid] ![](http://folksy.com/images/folksy-colour.png) ![](http://folksy.com/images/folksy-colour2.png) ![](http://folksy.com/images/folksy-colour3.png) [/grid]`, - { siteSettings: { experimental_post_image_grid: true } }, `



@@ -1784,12 +1774,11 @@ var bar = 'bar'; "image grid with 3 images works" ); - assert.cookedOptions( + assert.cooked( `[grid] ![](http://folksy.com/images/folksy-colour.png) ![](http://folksy.com/images/folksy-colour2.png) ![](http://folksy.com/images/folksy-colour3.png) [/grid]`, - { siteSettings: { experimental_post_image_grid: true } }, `


@@ -1797,9 +1786,8 @@ var bar = 'bar'; "image grid with mixed block and inline images works" ); - assert.cookedOptions( + assert.cooked( "[grid]![](http://folksy.com/images/folksy-colour.png) ![](http://folksy.com/images/folksy-colour2.png)[/grid]", - { siteSettings: { experimental_post_image_grid: true } }, `

`, diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-grid.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-grid.js index 7e6b2626399..c55b3543fa3 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-grid.js +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-grid.js @@ -11,17 +11,9 @@ const gridRule = { }; export function setup(helper) { - helper.registerOptions((opts, siteSettings) => { - opts.enableGrid = !!siteSettings.experimental_post_image_grid; - }); - helper.allowList(["div.d-image-grid"]); helper.registerPlugin((md) => { - if (!md.options.discourse.enableGrid) { - return; - } - md.block.bbcode.ruler.push("grid", gridRule); }); } diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 63d364b9e7b..71bb90ae97b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2430,7 +2430,6 @@ en: experimental_new_new_view_groups: 'EXPERIMENTAL: Enable a new topics list that combines unread and new topics and make the "Everything" link in the sidebar link to it.' enable_custom_sidebar_sections: "EXPERIMENTAL: Enable custom sidebar sections" experimental_topics_filter: "EXPERIMENTAL: Enables the experimental topics filter route at /filter" - experimental_post_image_grid: "EXPERIMENTAL: Enables a [grid] tag in posts to display images in a grid layout." experimental_search_menu_groups: "EXPERIMENTAL: Enables the new search menu that has been upgraded to use glimmer" errors: diff --git a/config/site_settings.yml b/config/site_settings.yml index f21a88baeeb..9244d50286c 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2112,9 +2112,6 @@ developer: experimental_topics_filter: client: true default: false - experimental_post_image_grid: - client: true - default: false new_edit_sidebar_categories_tags_interface_groups: type: group_list list_type: compact