OEL-3343: Copyright overlay. #430

tibi2303 · 2024-09-11T23:47:37Z

Jira issue(s):

D8AGE-???

drishu

~~when I click the copy button, it gives no feedback, feels strange~~ its fixed

templates/patterns/copyright_overlay/copyright_overlay.ui_patterns.yml

templates/patterns/copyright_overlay/pattern-copyright-overlay.html.twig

resources/js/copy_clipboard.js

templates/patterns/copyright_overlay/pattern-copyright-overlay.html.twig

resources/sass/components/_copyright_overlay.scss

tests/src/Kernel/fixtures/markup_rendering_patterns/copyright_overlay.yml

templates/patterns/copyright_overlay/copyright_overlay.ui_patterns.yml

brummbar

We need to write a JS test to test the functionality alone. Use the pattern preview page for the test.

brummbar · 2024-09-18T12:09:31Z

resources/sass/components/_copyright_overlay.scss

@@ -0,0 +1,20 @@
+.copyright-overlay {


This needs to be a dedicated library to attach to the component alone.

This library will have the standalone js and the copy_clipboard one as dependency.

done, have a look, i hope i understood corectly

templates/patterns/copyright_overlay/pattern-copyright-overlay.html.twig

brummbar · 2024-09-18T12:22:41Z

templates/patterns/copyright_overlay/pattern-copyright-overlay.html.twig

+    'attributes': create_attribute()
+      .setAttribute('id', modal_id)


Suggested change

'attributes': create_attribute()

.setAttribute('id', modal_id)

'attributes': create_attribute({

id: modal_id

})

brummbar · 2024-09-18T13:38:50Z

resources/js/copy_clipboard.js

+   */
+  Drupal.behaviors.copyClipboard = {
+    attach: function (context) {
+      Array.prototype.forEach.call(document.querySelectorAll('[data-copy-target]'), function (element) {


Suggested change

Array.prototype.forEach.call(document.querySelectorAll('[data-copy-target]'), function (element) {

once('oebt-clipcopy', '[data-copy-target]', context).forEach(function (element) {

And of course we need to add once library to the dependencies.

brummbar · 2024-09-18T15:09:23Z

resources/js/copy_clipboard.js

+    detach: function (context, settings, trigger) {
+      if (trigger === 'unload') {
+        Array.prototype.forEach.call(document.querySelectorAll('[data-copy-target]'), function (element) {
+          element.removeEventListener('click');


Here we remove all click events. To be more accurate, we should remove only our click event. In order to do so, you need to pass the same callback that you used in the addEventListener method. It's enough to name the function and put it in the current scope.

brummbar · 2024-09-18T15:09:49Z

resources/js/copy_clipboard.js

+
+    detach: function (context, settings, trigger) {
+      if (trigger === 'unload') {
+        Array.prototype.forEach.call(document.querySelectorAll('[data-copy-target]'), function (element) {


This needs to use context.

oe_bootstrap_theme.libraries.yml

brummbar · 2024-09-18T17:11:18Z

resources/js/copy_clipboard.js

+    attach: function (context) {
+      Array.prototype.forEach.call(document.querySelectorAll('[data-copy-target]'), function (element) {
+        var targetClass = element.getAttribute('data-copy-target');
+        var targetElement = document.querySelector('.' + targetClass);


One big problem here: if there are two copyright elements with the same class, the first one in the page will be always copied. If we want to be similar to bootstrap, let's just use the whole selector for the query, and leave the choice to the user. Otherwise we need to make sure to use an ID. Maybe ID in this case is better, or users won't know and pass a class and encounter the same issue.

changed.. now it has the whole selector as attr

oe_bootstrap_theme.libraries.yml

resources/js/copy_clipboard.js

tests/src/FunctionalJavascript/CopyClipboardTest.php

resources/js/copy_clipboard.js

brummbar

Looks good, one remark and one question.

brummbar · 2024-09-26T16:26:19Z

includes/pattern.inc

+ */
+function oe_bootstrap_theme_preprocess_pattern_copyright_overlay(array &$variables): void {
+  $variables['modal_id'] = Html::getUniqueId('bcl-copyright-modal');
+  $variables['copy_selector'] = Html::getUniqueId('copy-selector');


I would use copyright-content as ID.

brummbar · 2024-09-26T16:27:36Z

resources/js/copy_clipboard.js

+  const handleClick = function (event) {
+    const element = event.currentTarget;
+    const targetSelector = element.getAttribute('data-copy-target');
+    const targetElement = document.querySelector(targetSelector);


So we decided to go for the approach where is the user that needs to ensure to pass a selector that triggers one element only?

yes, i think it's better because we let the user make extended selectors and it's up to them to provide the correct one.

drishu requested changes Sep 13, 2024

View reviewed changes

drishu requested changes Sep 16, 2024

View reviewed changes

drishu reviewed Sep 16, 2024

View reviewed changes

templates/patterns/copyright_overlay/copyright_overlay.ui_patterns.yml Show resolved Hide resolved

drishu reviewed Sep 16, 2024

View reviewed changes

templates/patterns/copyright_overlay/copyright_overlay.ui_patterns.yml Show resolved Hide resolved

drishu previously approved these changes Sep 16, 2024

View reviewed changes

brummbar requested changes Sep 18, 2024

View reviewed changes

tibi2303 dismissed drishu’s stale review via 9e98df0 September 23, 2024 09:36

tibi2303 added 10 commits September 24, 2024 10:27

OEL-3343: Copyright overlay.

ca889ba

OEL-3343: Removed unused param.

953dee4

OEL-3343: Improvements on copyright.

e42fa6c

OEL-3343: Added also title param.

fd86819

OEL-3343: Overlay tests fixes and changed to rem approach.

6cb6883

OEL-3343: Changed to standalone pattern and added js tests.

8d84145

OEL-3343: Fixed tests.

c1f9e94

OEL-3343: Added selenium to drone.

9f3b5e3

OEL-3343: Updated markup tests.

e94a2b7

OEL-3343: Grump code.

30e0bd9

tibi2303 force-pushed the OEL-3343 branch from bfceadd to 30e0bd9 Compare September 24, 2024 07:27

drishu requested changes Sep 24, 2024

View reviewed changes

tibi2303 added 2 commits September 25, 2024 14:29

OEL-3343: Bug fixing copy and improvement on comments.

5a19126

OEL-3343: Updated tests.

96480b2

tibi2303 force-pushed the OEL-3343 branch from 2dafae8 to 96480b2 Compare September 25, 2024 13:06

drishu added 8 commits September 25, 2024 19:00

OEL-3343: Improve readability with heredoc.

848ade3

OEL-3343: Remove detach trigger filter.

9254952

OEL-3343: Use once js in detach for consistency.

fddda35

OEL-3343: Remove redundant port expose.

531fe2e

OEL-3343: Remove redundant template variables.

dc58ec7

OEL-3343: Harmonize attribute creation.

2b124ba

OEL-3343: Prefix preview.

ebe82c3

OEL-3343: Test both copyright elements.

ac82f43

drishu added 3 commits September 26, 2024 09:55

OEL-3343: Improve example texts.

c6cf835

OEL-3343: Remove redundant space.

6ddfe8e

OEL-3343: Add test for attribute override.

225cffe

drishu requested changes Sep 26, 2024

View reviewed changes

resources/js/copy_clipboard.js Outdated Show resolved Hide resolved

OEL-3343: Diversify testing texts.

f81998d

drishu previously approved these changes Sep 26, 2024

View reviewed changes

brummbar requested changes Sep 26, 2024

View reviewed changes

OEL-3343: Changed to copyright-content as unique ID.

815722c

tibi2303 dismissed drishu’s stale review via 815722c September 27, 2024 07:27

OEL-3343: Updated markup tests.

6495de2

brummbar approved these changes Sep 27, 2024

View reviewed changes

drishu approved these changes Sep 27, 2024

View reviewed changes

tibi2303 changed the base branch from 1.x to EPIC/OEL-3442 September 30, 2024 08:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OEL-3343: Copyright overlay. #430

OEL-3343: Copyright overlay. #430

tibi2303 commented Sep 11, 2024

drishu left a comment •

edited

Loading

brummbar left a comment

brummbar Sep 18, 2024

brummbar Sep 18, 2024

tibi2303 Sep 24, 2024

brummbar Sep 18, 2024

tibi2303 Sep 24, 2024

brummbar Sep 18, 2024

brummbar Sep 18, 2024

tibi2303 Sep 24, 2024

brummbar Sep 18, 2024

tibi2303 Sep 24, 2024

brummbar Sep 18, 2024

tibi2303 Sep 24, 2024

brummbar Sep 18, 2024

tibi2303 Sep 24, 2024

brummbar left a comment

brummbar Sep 26, 2024

tibi2303 Sep 27, 2024

brummbar Sep 26, 2024

tibi2303 Sep 27, 2024

		'attributes': create_attribute()
		.setAttribute('id', modal_id)

	Array.prototype.forEach.call(document.querySelectorAll('[data-copy-target]'), function (element) {
	once('oebt-clipcopy', '[data-copy-target]', context).forEach(function (element) {

OEL-3343: Copyright overlay. #430

Are you sure you want to change the base?

OEL-3343: Copyright overlay. #430

Conversation

tibi2303 commented Sep 11, 2024

drishu left a comment • edited Loading

Choose a reason for hiding this comment

brummbar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brummbar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drishu left a comment •

edited

Loading