Hello @Hritick,
The issue was raised against the version 1.0.0-beta1 of the theme, hence the MR should be raised against the 1.x branch only, not directly against the main branch!
Please change the target branch accordingly
Hi @Tirupati_Singh,
Thanks for putting efforts on making the recommended changes. The changes seems reasonable to me now, I've also applied the modified patch and can verify that it solves the issue, hence moving it to RTBC++
Attaching before after screenshots for reference!
Hello @Tirupati_Singh,
Thanks for the update, now I can successfully reproduce the issue. I've also cross checked whether the issue is arising on different themes as well or not, and as its only occurring with Gin theme,hence we can conclude that the issue of the Gin theme only and not the module which is being used here. I'll attach all the screenshots for reference.
I've tried to apply your patch, though I haven't encountered any error on patch application, but unfortunately the patch is not helping to resolve the issue as well. So moving it back to NW state once again!
Also as a suggestion I'd like to add that there should also be a breathing space b/w the checkbox and the image as well, as its there for other themes, and let's not only restrict for smaller images/icons. We should design in a such a way so that it doesn't seem to break irrespective of the image size!
Hello there,
The steps to reproduce is not clearly mentioned over here, and I've digged up a lot, but haven't found any way to reproduce the issue.
Can anyone kindly help me reproduce the issue, so that I can test the patch and help the issue move forward.
Also the issue summary needs to be updated accordingly then, hence moving it to NW for now!
Thanks!
Hi, @Tanushree Gupta the patch is not getting applied now, throwing error. Please check!
These are the errors I'm currently getting while applying the patch:
Checking patch css/layout/layout.css...
error: while searching for:
border: 1px solid #0fbe7c;
text-decoration: none;
}
error: patch failed: css/layout/layout.css:1295
error: css/layout/layout.css: patch does not apply
Checking patch css/layout/layout.css...
Hunk #1 succeeded at 839 (offset -1 lines).
error: while searching for:
.search-block-form input {
margin: 0;
padding: 0.6em 1em;
width: 100%;
}
error: patch failed: css/layout/layout.css:950
error: css/layout/layout.css: patch does not apply
Checking patch css/state/responsive.css...
Checking patch insha.libraries.yml...
Checking patch js/script.js...
Checking patch templates/page.html.twig...
headerfix.patch:344: new blank line at EOF.
+
Checking patch templates/region--primary-menu.html.twig...
Checking patch css/layout/layout.css...
error: while searching for:
display: flex;
justify-content: space-between;
align-items: center;
/* box-shadow: 0 0 60px 0 rgba(0,0,0,.1) */
}
.region-primary-menu {
error: patch failed: css/layout/layout.css:1268
error: css/layout/layout.css: patch does not apply
Checking patch templates/page.html.twig...
Hello @silvi.addweb,
Thanks for the efforts. After your recent commit the patch completely resolves all the remaining Phpcs issues.
Hence moving it to RTBC++
Hi @ufku,
Now I got what was causing the mismatch between your findings and mine. Basically there was version mismatch of php_codesniffer and coder and we were running the standards through. Now updated my version and can confirm no errors are there.
But one thing caught my attention while going through the artifacts for Phpcs check: https://project.pages.drupalcode.org/-/imce/-/jobs/1819695/artifacts/jun...
Here I can see we're scanning only the php files in terms of the extension of .php, .module, .install on contrary to that the job is running over Drupal and DrupalPractice coding standards which incorporates other types of file as well like the css file, .md, .yml file etc. So wondering what's the reason for this anomaly?
Also I can see we're not checking for js files at all, as it doesn't get checked by default by Phpcs. I'm aware that the eslint will be a more robust tool for finding and fixing js errors but as per my findings mentioned on thread
#7
🐛
Fix the errors/warnings reported by PHP_CodeSniffer.
Fixed
Phpcs now uses core's eslint config when we're explicitly forcing Phpcs to check for Js errors. Now as we don't have any custom eslint configured specifically for the module, we can use core's eslint config to address some common issues, and later on if the module incorporates its own eslint config we can move to that certainly.
As per as the CSS errors are concerned, Phpcs and Phpcbf comes pretty handy to address those, here we're not even using SCSS or POSTCSS so it shouldn't cause any issue as well!
Can you kindly enlighten me on these?
Hello @Ashutosh Ahirwal,
I've made changes according to your suggestions by adding another commit to the MR!1 . Please have a look now!
Hello @Tanushree Gupta,
I've checked your latest changes and on applying the patch though the tab styling and form styling looks good now, but I found other problems also, which I think goes beyond the scope of the issue.
As far as this issue is concerned, most the things have been addressed well, and you're correct that we can manipulate the placement of blocks through UI itself!
But a small issue seems to be there in terms of styling, which might not be possible to fix through UI, and need some code changes.
When placing 'Site Branding' and 'Primary Tabs' block on the Header region its showing closely packed, we can turn it into a flexbox display and provide a breathing space!
Attaching a screenshot for quick reference!
Also, after applying the patch over the latest release, though it solves most of them as already RTBCed, but now we're getting some errors as well, so we need to incorporate those fixes as well over here and solve the merge error!
FILE: .../web/modules/contrib/imce/js/plugins/ckeditor5/imce.ckeditor5.js
----------------------------------------------------------------------
FOUND 15 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
31 | ERROR | [x] Expected 1 space before "?"; 0 found
31 | ERROR | [x] Expected 1 space after "?"; 0 found
31 | ERROR | [x] Expected 1 space before "?"; 0 found
31 | ERROR | [x] Expected 1 space after "?"; 0 found
31 | ERROR | [x] Expected 1 space before "?"; 0 found
31 | ERROR | [x] Expected 1 space after "?"; 0 found
33 | ERROR | [x] Expected 1 space before "?"; 0 found
33 | ERROR | [x] Expected 1 space after "?"; 0 found
33 | ERROR | [x] Expected 1 space before "?"; 0 found
33 | ERROR | [x] Expected 1 space after "?"; 0 found
35 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
38 | ERROR | [x] Expected 1 space before "?"; 0 found
38 | ERROR | [x] Expected 1 space after "?"; 0 found
38 | ERROR | [x] Expected 1 space before "?"; 0 found
38 | ERROR | [x] Expected 1 space after "?"; 0 found
----------------------------------------------------------------------
Hence moving it to NW state!
Hello @ufku,
Went through your commits just a while ago and noticed its doing some optimisation and coding standard fixes as well, but I'm afraid that it doesn't addresses the Phpcs reported issues.
The issue is talking about the errors that are being reported by Phpcs and not other code optimisations, hence I think it would've been better if we had been limited to the issue title only, as it'll be more transparent in future to track changes.
Not sure why this issue got closed, when you haven't merged the MR which was addressing the same. As a result all the Phpcs issues still persist on the latest tag.
Merger of the MR could've rightfully solved it.
It also came to my notice that the recent commits are failing some tests as well along with some warnings, apart from that as the changes have been made directly onto the target branch, resulting in a merge error for the existing MR.
Would love to hear from you soon. Till then I'm reopening the issue as its still reporting the Phpcs issues:
FILE: /app/web/modules/contrib/imce/js/imce.js
----------------------------------------------------------------------
FOUND 71 ERRORS AFFECTING 69 LINES
----------------------------------------------------------------------
274 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
276 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
393 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
439 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
542 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
545 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
553 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
587 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
621 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
632 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
675 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
676 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
679 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
680 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
683 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
701 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
705 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
714 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
716 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
725 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
728 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
737 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
739 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
748 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
754 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
759 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
761 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
770 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
777 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
779 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
786 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
789 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
791 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
793 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
919 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
919 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
920 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1039 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
1039 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
1040 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1089 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
1138 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
1180 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1185 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1199 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
1234 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1274 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
1289 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
1297 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1319 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1336 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1337 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
1338 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1342 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1368 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1373 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
1406 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1474 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
1488 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1497 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1596 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
1615 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1619 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1772 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
1831 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1857 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1883 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
1954 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
1976 | ERROR | [x] Functions must not contain multiple empty lines
| | in a row; found 2 empty lines
2041 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
2044 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 71 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/imce.input.js
----------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
26 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
34 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
49 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
54 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
98 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/imce.Tbb.js
----------------------------------------------------------------------
FOUND 9 ERRORS AFFECTING 9 LINES
----------------------------------------------------------------------
90 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
94 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
114 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
150 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
155 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
160 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
175 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
179 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
208 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 9 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/imce.filefield.js
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
76 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
84 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/imce.Folder.js
----------------------------------------------------------------------
FOUND 31 ERRORS AFFECTING 31 LINES
----------------------------------------------------------------------
32 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
187 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
235 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
241 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
268 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
273 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
313 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
344 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
369 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
375 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
395 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
397 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
398 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
410 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
418 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
551 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
576 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
587 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
620 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
623 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
676 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
681 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
685 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
691 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
695 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
720 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
745 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
752 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
754 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
772 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
786 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 31 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/imce.File.js
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
31 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
57 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: .../web/modules/contrib/imce/js/plugins/ckeditor5/imce.ckeditor5.js
----------------------------------------------------------------------
FOUND 17 ERRORS AFFECTING 6 LINES
----------------------------------------------------------------------
31 | ERROR | [x] Expected 1 space before "?"; 0 found
31 | ERROR | [x] Expected 1 space after "?"; 0 found
31 | ERROR | [x] Expected 1 space before "?"; 0 found
31 | ERROR | [x] Expected 1 space after "?"; 0 found
31 | ERROR | [x] Expected 1 space before "?"; 0 found
31 | ERROR | [x] Expected 1 space after "?"; 0 found
33 | ERROR | [x] Expected 1 space before "?"; 0 found
33 | ERROR | [x] Expected 1 space after "?"; 0 found
33 | ERROR | [x] Expected 1 space before "?"; 0 found
33 | ERROR | [x] Expected 1 space after "?"; 0 found
35 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
38 | ERROR | [x] Expected 1 space before "?"; 0 found
38 | ERROR | [x] Expected 1 space after "?"; 0 found
38 | ERROR | [x] Expected 1 space before "?"; 0 found
38 | ERROR | [x] Expected 1 space after "?"; 0 found
91 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
127 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 17 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/plugins/imce/imce.upload.js
----------------------------------------------------------------------
FOUND 19 ERRORS AFFECTING 19 LINES
----------------------------------------------------------------------
39 | ERROR | [ ] String concat is not required here; use a single
| | string instead
40 | ERROR | [ ] String concat is not required here; use a single
| | string instead
42 | ERROR | [ ] String concat is not required here; use a single
| | string instead
45 | ERROR | [ ] String concat is not required here; use a single
| | string instead
47 | ERROR | [ ] String concat is not required here; use a single
| | string instead
86 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
94 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
98 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
112 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
116 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
119 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
126 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
136 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
146 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
150 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
153 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
215 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
230 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
235 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 14 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/plugins/imce/imce.resize.js
----------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
37 | ERROR | [ ] String concat is not required here; use a single
| | string instead
40 | ERROR | [ ] String concat is not required here; use a single
| | string instead
43 | ERROR | [ ] String concat is not required here; use a single
| | string instead
74 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
102 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/plugins/imce/imce.newfolder.js
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
48 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
59 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
63 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
65 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: ...app/web/modules/contrib/imce/js/plugins/imce/imce.UploadQueue.js
----------------------------------------------------------------------
FOUND 17 ERRORS AFFECTING 17 LINES
----------------------------------------------------------------------
61 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
62 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
73 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
74 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
141 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
197 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
198 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
206 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
262 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
276 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
291 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
308 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
316 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
333 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
352 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
354 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
361 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 17 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: ...pp/web/modules/contrib/imce/js/plugins/bueditor/imce.bueditor.js
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
54 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/js/imce.Item.js
----------------------------------------------------------------------
FOUND 12 ERRORS AFFECTING 11 LINES
----------------------------------------------------------------------
370 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
392 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
411 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
428 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
428 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
443 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
458 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
470 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "NULL" but found "null"
486 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
489 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
505 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
516 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/contrib/imce/css/imce.icons.css
----------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
5 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found
| | 4
5 | ERROR | [ ] No style definition found on line; check for missing
| | colon
6 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found
| | 4
6 | ERROR | [ ] No style definition found on line; check for missing
| | colon
7 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found
| | 4
7 | ERROR | [ ] No style definition found on line; check for missing
| | colon
----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Time: 679ms; Memory: 34MB
Hello, @Anita Verma
Nothing has changed after your commit unfortunately, the issue still persist and I can see most of the MR review comments have not been addressed at all.
Hence keeping it on NW state as transitioned correctly by @pray_12
Hello Tirupati_Singh,
Yes you're right, the error on the PATCHES.txt is coming from cweagan composer dependency and it'll be dynamically generated everytime on applying patch via composer, so its not an issue of this module.
The other one which was there, is now resolved after the new commit.
Hence moving it to RTBC++
Hello there,
All the MR comments seems to be resolved now, and after applying the patch the issue has also been resolved!
Though it seems that the issue still persist on the latest version of the theme as well, and I believe after merging it'll be resolved. So changing the meta data component accordingly.
Moving it to RTBC++ with relevant screenshots!
Hello @Gaurav Gupta,
Seems like the issue has been resolved in the latest version of the theme. Hence closing the issue as of now.
If you land upon any other findings, feel free to reopen it!
Hello @Hritick,
I went through the theme and followed the steps as you've mentioned on your previous comment. I see there's a legit issue with it because the region is getting rendered twice from page.html.twig. Though I feel that can be addressed on a separate issue. It'll be good to stay in the scope of the issue's motivation.
I've applied the patch provided over here and can confirm that after applying it, the left side spacing issue is getting resolved, also previously there was a horizontal scrollbar coming, after applying the patch that issue has also been resolved.
Also I've checked this issue persist on the latest version of the theme as well, so I'm changing the metadata accordingly!
For the footer getting rendered twice please feel free to create a sub issue of this, we can have the discussion over there pertaining to that problem.
As the said issue is resolved, hence I'm moving it to RTBC++, attaching before and after screenshots for reference!
Hello @kaaarrrtik,
I've appllied your patch and tested the same, and looks like nothing is working here. The multilevel, menu is getting shown on hover or on click of the dropdown button, on responsive screens the harburger menu is working as well.
I've added quite a few comments on the MR itself, pls try to address them!
Attaching the screen recording for a quick reference..
Hello @Tanushree Gupta,
I've reviewed your MR, though the MR seems to be good and after applying the patch the header style on active status is getting resolved and menu links are properly visible, but I don't think it still looks good. The background should cover the width of the menu items and not the full width, also on placing the site-branding block on the primary menu region things are getting broken, I things blocks in the regions should also be in flex, so as to make it appealing. On the other hand, I've identified that when I'm placing the site-branding block on the header region its not getting visible. So I think it'll be better if we target that as well over here, as we're talking about the whole header styling.
Attaching all the screenshots for reference, also the issue metadata seems to be wrong, hence updating that as well!
Hi,
I've encountered the aforementioned issue, and after applying both the patches the issue is getting resolved.
Reviewed both the MR, both the MRs are looking good to me, but MR!23 provides a configuration option from the theme settings from which it lets the user to choose the number of slides, which not only solves the card rendering issue on the homepage, but also dynamically renders card components on the theme settings form based on the chosen value, for which it seems more of a robust solution to me!
Attaching before and after screenshots for reference!
Hello there,
This seems to be a support request and not a feature request. Also its quite natural that different themes will incorporate different styles, so no MR is needed here.
Hence closing the issue, by changing the metadata properly!
Hello there,
The changes looks good to me! Hence moving it to RTBC++
Hello there,
Yes its true that the issue was there, but after applying the patch the issue is resolved now. And also the MR looks good to me.
Hence moving it to RTBC++
Attaching before after screenshot for reference!
Hello,
I think there's no problem regarding this in the theme, as different blocks are placed on the sidebar_first region that's why the space was getting occupied. The page.html.twig also correctly implements conditional rendering for the sidebar blocks. Hence, if anyone wants to show their main content in full width view, they must place blocks according to that in specified regions defined in the theme, which can be easily manipulated from UI in the block layout.
As per my configuration the search form was getting rendered initially on the sidebar, as a result the space was getting divided b/w the main content and the sidebar and on removal of that the main content is taking full width.
So as per my findings there's no issue in terms of styling of twig's conditional rendering.
Hence as of now I'm closing the issue. Please feel free to reopen the issue, if you land upon any other conclusion.
Attaching both the screenshots for reference
Thanks, @Tanishree Gupta for the quick action.
I can see all the comments have been addressed now. After applying the patch the image is not blurry or pixelated now anymore.
Hence moving it to RTBC++
Attaching screenshot for reference!
@Tanushree Gupta, Now the changes look fine to me and all the comments have been addressed as I can see. But I noticed that here the CTA buttons are actually a part of the image and not actually a button. The title, the CTA buttons and the side thumbnail image all are part of the banner image, but it should not be like this, only side image should be considered as image and other should be considered as per their standard usage. Though its beyond the scope of this issue, hence moving it to RTBC++
Hello, @Tanushree Gupta,
After applying the patch the navbar region is getting rendered on the page as expected. Though there's no styling on the menu and the header as well, but I think that's beyond the scope of this ticket. Hence moving it to RTBC++
Attaching screenshots for reference!
Hi @Tanushree Gupta,
After applying your patch the horizontal scrollbar issue is resolved, but I can see other several style breaks in the login page.
It'll be great if we can address them in this issue as well.
On small viewports as that of iPhone15 or so the search block is overlapping with nav tabs, we can make it responsive, in my opinion all the tabs and buttons can have a little bit more padding, otherwise it looks to be too shrunken.
Btw, I cannot see any placeholder or labels on the form, it should be there to make the theme more accessible to assistive technologies like screen-readers. I'd prefer a placeholder over label, just a personal preference though!
On smaller viewports the field names are coming as horizontally centered whereas on desktop view its left aligned, so I think we should follow the same for smaller viewports as well to maintain cohesiveness through out the design.
Attaching relevant screenshots for reference!
I took a deeper dive onto the issue of the twitter logo of this module! But as all the CSS and JS responsible for that is coming from a CDN and not only that there're multiple references like all the CSS is coming and getting build from the JS files only, on which the logo has been added as HTC component using foreign CSS property introduced by MS as an hack, which is basically a CSS3 Pie Polyfill. The HTC component is also coming from external sources and along with that the actual image which has been referred to the htc component is also coming from an external source.
So in this case, as we're dependent on multiple external sources, we can't do anything unless all the sources gets updated, we don't have any flexibility to make changes from the codebase. I've also searched for latest CDNs and HTC components from their authorised tool, but no luck in that also.
Apart from that, the hack was specifically meant to bring better support on IE for images predominantly. In 2024 I think we're not that much concerned for compatibility for IE, in that scenario I think it'll be an overkill and increment of redundant code which in turn might affect the web vitals adversely.
So basically we need to wait for the maintainer to update the external references. I think it'll be better if we avoid such multiple external references now, as it's reducing the flexibility of the developer to customise the same.
sourojeetpaul → created an issue.
Hi,
@Tirupati_Singh, I've applied your patch on the specified version of the module, apart from the js issues I'm also getting two more issues which are being reported by Phpcs.
FILE: /app/web/modules/contrib/imce/PATCHES.txt
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
5 | ERROR | [x] Expected 1 newline at end of file; 3 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: ...modules/contrib/imce/tests/src/Kernel/CKE4to5UpgradePathTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
3 | ERROR | [x] Expected strict_types=1, found strict_types = 1.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Time: 987ms; Memory: 34MB
And as far as the errors regarding the js files are concerned, I took a deeper dive into the issue, and discovered that the coding standards that we're passing over here or in general are Drupal and DrupalPractice which doesn't scan the js files by default as the Drupal coding standard is limited to the scanning of these files : php,module,inc,install,test,profile,theme,css,info,txt,md,yml and DrupalPractice scans these following extensions: php,module,inc,install,test,profile,theme,yml
Both of them are not considering js files by dafault, so we need to include js file explicitly.
Ref issue: https://github.com/acquia/blt/issues/2861
And on another issue of D.C it has been stated that eslint should be the preferred tool for finding and fixing js issues, and I also do belief the same, as it provides a more robust way to define rulesets and getting access to a wide repository of pre-defined rulesets like that of airbnb, airbnb-base etc, along with that we can define our custom rulesets or develop custom plugins which can be more fruitful in streamlining dev process.
Ref link:
https://www.drupal.org/project/drupal/issues/2975201 →
But I got confused on seeing this: https://github.com/acquia/blt/issues/2861
Where it has been stated that Phpcs can now sniff and lintJs by conforming to the rulesets defined in Core's .eslintrc file. But the issue over that is, it seems that Phpcs is not correctly referring to the Core's eslint rulesets.
Ref link: https://github.com/acquia/blt/issues/2845
After all these RnD, I believe that it'll be better to ignore these Js issues as of now, and only target the remaining issues and then it'll be more robust to prepare a module specific .eslintrc file and then running the js linting task over that specified rule. Though for that we need to discuss with the module maintainer and raise a new ticket for the same, as it goes beyond the scope of Phpcs fixes!
Hence adding the label of 'Needs subsystem maintainer review' and moving it to NW state for fixing the remaining two issues!
Hello there,
The MR looks good to me and after applying the patch the theme is now compatible and installable on D10.
Attaching before and after screenshots for reference!
Hello there,
I've reproduced the issue on the specified version and tested the patch also. The MR solves the spacing issue of the action buttons.
But on the latest version the issue doesn't persist, I think it has been already addressed! Though on the latest version I've found some other style break issues related to the action button and filters. I think it'll be great if we address those in this!
I'm attaching MR testing screenshots as well as the screenshots for the newly found style break issues as well as some accessibility issues are there!
Hence I'm changing the version component on the metadata!
Hello, @Tirupati_Singh,
Good observation! In that case I think its going beyond the scope of this issue ticket, and we can have a separate issue to address this!
As I've mentioned in my previous comment, apart from that, everything looks fine to me, hence moving it to RTBC++
Also, @Nupur Badola the issue is related to media and media library and can only be reproduce by adding media reference to a CT.
I see you're trying to test the same on normal image field, which is not pertaining to this issue, though its not clear to me what you tried to decipher through the video. If you find any issues with the image field and if it seems to be related to the base theme Claro and not restricted to Gin theme, then in my opinion it'd be better if we move that discussion by creating a new issue on Claro!
Hi, @Nupur Badola
The 403 error that you're encountering I believe that its an issue with your setup and Nginx server configuration. Please check your Nginx error log to debug further.
Also @Tirupati_SIngh, I've tested your patch, though it solves the colour consistency issue and the close icon appearance, but the the point which was raised by @shweta__sharma has not been addressed, as I cannot see the close icon on normal image fields, as she already mentioned, it was only appearing on the Media modal!
It'll be great if we can achieve design consistency over here as well! Hence keeping it in NW state as of now.
I'm attaching before and after screen recordings for the same for easy reference!
Hi, @Tirupati_Singh
I've successfully reproduced the issue by follwing the 'Steps to reproduce' and encountered the same, and after applying the patch the issue gets resolved, and clearly appear on Bootstrap theme, also the previous workings are not getting affected for that, which I've cross-checked on Gin, CLaro and Olivero theme as well.
Hence moving it to RTBC++
Also attaching relevant screenshots for reference!
Hi, @Tirupati_Singh
After applying your patch the module is now compatible with D10, I've also reviewed your MR and the changes looks fine to me.
It correctly incorporates Drupal once function and correct namespaces along with version requirement.
Hence moving it to RTBC++
Attaching before and after screenshots for reference!
Hi, @Tirupati_Singh,
After applying your patch the theme is now compatible with D10 and can be installed successfuly.
I've also reviewed the MR, it looks good, the js code has been correctly incorporated with Drupal Behaviours, the deprecated functions on .theme file has also been addressed properly.
Hence moving it to RTBC++.
Attaching the before and after screenshots for reference!
Hi, although @Kanchan Bhogade hav tested and confirmed that its working fine with the patch but as the issue is still in NR state, I had to perform re-test of the same.
On my test on D10 instance the issue gets fixed with the MR, and the changes seems to be reasonable also, hence moving it to RTBC++
Also attaching the before and after states for reference!
Hi, @Esha_Kundu
The issue you've mentioned still persists, but after applying your patch I can see the screenshot now.
Hence moving it to RTBC++
Attaching before and after screenshots for reference!
@sourabhsisodia_,
The MR looks good, and after applying the patch the default logo is showing, But the logo is not properly visible for lesser contrast with the background slider.
But as the slider images can we anything based on users' choice, I think we can't do much over here.
Hence moving it to RTBC++
Also attaching before and after screenshots for reference!
Hi @joseph.olstad,
As I've mentioned in my previous comment, even I couldn't reproduce the issue on my Drupal instance, I tried to reproduce the same on Drupal 10.2 and the specified version of the module, but haven't encountered any such issues.
But the MR looks good to me, and it incorporates the once() function, which should be the preferable approach in such cases.
I applied the patch and tested it thoroughly, nothing seems to be breaking on applying it!
Though, it'll be better if we can add 'Steps to Reproduce' in a more detailed manner, which is absolutely missing now!
@dipakmdhrm can you pls attach some screenshots or screen recordings of the issue that you're encountering.
Rest the MR looks good and we can ship it!
The MR looks good and solves the issue.
Attaching before and after screenshots for reference!
Hence moving it to RTBC++
Hi @Agamich, I'v also encountered the gulp package deprecation issue, and tried to apply your patch, but even after applying the patch some issues still persists. Also I think it'll be better to raise an MR for the same, as the community prefers MR over patches.
So I took a closer look and solved some remaining issues.
Updated the nvmrc to the latest version, also there were some issue in the .eslintrc.json due to some missing plugins, some config dependency related to the airbnb-base was still missing and looked like the config of gulp-eslint is also broken, several people reported the same, also its not that much well maintained nowadays, as a result its encounterimg some issues while building the dep graph So a better option will be to opt for the latest gulp-eslint-new plugin.
You can refer to this issue for the same: https://github.com/airbnb/javascript/issues/2545
I've incorporated all these in changes on MR!215
Please have a look!
After these changes eslint is correctly referring to some ES6 Module errors, which needs to fixed. I think that's beyond the scope of this issue and perhaps we can address that after proper merger of this one.
sourojeetpaul → made their first commit to this issue’s fork.
Hello @Ashutosh, Haven't heard from from you for while now, regarding this.
Can you kindly clarify the confusion mentioned in the comment
#6
🐛
Need to design login page with tabs
Needs work
Hi,
I think running composer require 'drupal/classy:^1.0' after installation of the theme is not the desired solution, rather all its dependencies should be downloaded and installed along with it, and the latest commit addresses the same. Looks good to me!
So moving it to RTBC++
Attaching before after screenshots for reference!
Hi, @clarkssquared
After applying the patch that I provided I'm not getting any error. Also sorry for the confusion, the MR was actually raised for the dev version.
And as suggested by @nesta_ it'll be better if we solve issues in dev version which can then be checked and approved and ported to the beta version.
I see you're applying the patch in the beta version of the theme, So pls check once by applying the patch in dev version, I think the issues you're getting will be resolved. Once again sorry for the confusion regarding version.
@nesta_ On the branch list in gitlab I can see the parent branch only which is 9.1.x, there's no corresponding dev branch. Though there exist a tag for the same(9.1.x-dev), In general we raise MRs against branch and not the tag, as far as I know. Pls correct me if I'm wrong. So I'm changing the version component now to dev to avoid any further confusion.
Hi,
I've fixed the errors/warnings reported by Phpcs. Also noticed that the subtheme.info file was not correctly named, so renamed it so that it can be detected as a valid yml file!
Raised an MR addressing these. Please have a look:
https://git.drupalcode.org/project/da_vinci/-/merge_requests/2
Working on it!
sourojeetpaul → created an issue.
sourojeetpaul → changed the visibility of the branch 3397918-update-twitter-icon to hidden.
sourojeetpaul → changed the visibility of the branch 3397918-update-twitter-icon to active.
@Hamid.ali, Can you please confirm whether the issue gets resolved my the patch provide by me here!
MR !9
@Nupur Badola, I've mentioned the same on my last comment
#5
🐛
Twitter logo needs to be changed to the X
Needs review
The social icons section was made by placing a custom block. Please go through my findings once again!
@Nupur Badola, I styled it like this. As on click it'll rotate an transform into an "X" it seems to me that it looks better to not giving the full width to the last line, otherwise, all hamburgers will look similar. Just to have a different flavour!
@smustgrave, As far as I've tested nothing seems to be breaking! Attaching before and screenshits for reference.
Also seems like PhpUnit test is failing over here, but not sure why PhpUnit tests are failing when the MR only make chnages on PCSS and CSS files. Still trying to solve it by rebasing!
sourojeetpaul → made their first commit to this issue’s fork.
Hi, @bindu r, as you already tested this and works fine for you, can you kindly change the status to RTBC to help the ticket move forward!
I see its still in NR state!
Hi, I'm unable to reproduce the same, its throwing no error on my D10 instance.
Also checked the code and it seems to be there is no extra curly brace overe there on that block of code.
Working on it!
I'll have a look at this!
Hi @jrockowitz,
I think you might got confused by the first two MRs. Those were raised against Drupal Core, but later as per my findings I changed the fork repo to that of webforms and created a MR for the same against the branch 6.2.x which I suppose is for the latest version of the module.
And @dipakmdhrm also confirmed my findings and tested the same.
If it was a Drupal Core issue it should affect all other table headers like that of Core's View, but that is not the case here. Rather its a Webform issue and how its interacting the Core's themes. As mentioned on my previous comment, its behaving differently with differnt themes of Core, as of this I limited the scope to its interaction with Claro theme, maybe we can target its interaction with other themes in another issue.
I think the confusion was created as those previous two MRs were still open. Sorry for that! I couldn't close them as I'm not the one who opened those MRs respectively.
Please refer to MR !459 https://git.drupalcode.org/project/webform/-/merge_requests/459 for this issue. It targets the issue related to Webform module and how it interacts with Core's claro theme.
Also I'm hiding the previous MRs to avoid further confusion and changing the issue metadata once again!
sourojeetpaul → changed the visibility of the branch 3441015-sortable-column-titles to hidden.
sourojeetpaul → changed the visibility of the branch 3441015-sortable-columns-11.x to hidden.
Hi, @dipakmdhrm
I'm not able to reproduce the issue on my D10 instance with the latest version of the module as well as the latest vbo module in claro theme, but I think its always a good practice to use once() with the context in these type of circumstances, it'll prevent the said issue if it happens to occur in any case though, as I see that the once() function is not implemented in the latest version's js code, I personally feel that it'll be better to incorporate the same.
The MR looks good to me, and can be merged!
As I couldn't reproduce the same so I can't actually move it to RTBC, hence I'm changing the state to NR to help the ticket move forward!
CC: @joseph.olstad
sourojeetpaul → made their first commit to this issue’s fork.
Hello there,
I've tried to reproduce the issue on my D10 instance with the specified version of the gin theme and the latest version of clientside validation module(4.0.2), But I doesn't seem to reproduce the issue.
After enabling those module and setting gin as the admin theme, all the toggle buttons are appearing as expected.
Attaching a screenshot for reference.
Ok, I'll be looking into it!
Hello there,
It seems to be like an duplicate issue there are already two issue pertaining to the problem, I've commented on both of them and shared my findings.
Please head over to there and join the conversation. If you disagree with my findings/ land up on anything new feel free to change it accordingly and reopen the issue, as of now closing the same!
Link to the said issues:
https://www.drupal.org/project/field_group/issues/3318730
🐛
Drop button cut off at bottom in Field Group vertical tabs
Active
https://www.drupal.org/project/drupal/issues/3217868
🐛
Claro theme + field_group seems to hide content of dropbutton
Needs work
Hello there,
I've faced the same issue on my D10 instance and looked for the issue and landed upon another issue which seems to be stating this as an issue of Claro theme or the Paragraphs module, but after digging up quite a bit and trying it out with all possible combinations of theme and paragraphs module and field group module, it has come to by notice that the problem is neither with any of the themes nor with the paragraphs module, rather the issue is arising because of field group module.
I've talked about this elaborately there:
https://www.drupal.org/project/field_group/issues/3318730
🐛
Drop button cut off at bottom in Field Group vertical tabs
Active
https://www.drupal.org/project/field_group/issues/3318730#comment-15551915
🐛
Drop button cut off at bottom in Field Group vertical tabs
Active
Also attaching the same screenshots here for quick reference
On my observation the issue starts from the disappearance of the dropdown button itself and some structural changes which is happening because of the field group module. I'm not changing the issue component as there is already an issue with the same, so just adding that as a parent issue and related issue. I think it'll be better if we mark this as a duplicate issue as we're trying to address the same on
https://www.drupal.org/project/field_group/issues/3318730
🐛
Drop button cut off at bottom in Field Group vertical tabs
Active
If you land on any other findings feel free to change accordingly. Though updated the issue summary with my latest findings for a quick overview!
Also not sure how you're reproducing the issue on D11, cause I found that these said modules are not yet ready for D11.
Hi, @depakmdhrm Resolved the issues suggested by you in my latest commit. Please have a look!
https://git.drupalcode.org/project/webform/-/merge_requests/459
Hi, @Gauravvvv
As far as I know there is no hard and fast rules regarding the usage of theme. Its completely subjective. Though I've also seen Claro being used as admin theme predominantly, but one might choose to opt it for their frontend theme based on their needs. So I think it'll be better to address the issue and fix it, no matter if someone is using it as frontend theme or admin theme, we should fix the flaws in the respective theme!
Hello, @dipakmdhrm
The changes suggested by you are valid, and I've also observed that the webform results table behaves differently on different theme, Like this icon overlapping issue is appearing on the claro theme and there is another issue on the results table on viewing it with Olivero theme.
So I agree that the changes should be in that specific file, and resolving it right away!
Hi, I've finally resolved the sort icon overlap issue for long headings in webform results table and raised an MR for the same. Please have a look!
https://git.drupalcode.org/project/webform/-/merge_requests/459
This is how it looks now even with longer titles!
Hi, After digging up quite a bit, I found that the issue of overlapping of the icon is not with Drupal Core Views or Claro theme, rather its purely an issue of the webform module. At first I only tried to reproduce the issue with webforms module as mentioned by @dipakmdhrm, and hadn't tested it by creating a view. Sorry for that!
The issue seems to be legit when we are looking at webform submission sortable title but doesn't persist in other cases i.e views sortable column or so, The sort toggle icon is appearing properly in Claro theme. But while looking for that I encountered that there is an issue with the sort icon on Olivero theme in case of normal views. For a two column table if we make both the columns sortable then the sort icon as well as the sorting feature becomes available to the later one only irrespective of the length of heading, it disappears from the first column.
I think the issue of Olivero theme can be addressed on another issue or I'm not sure yet if any issue already exist in the D.O issue queue, have to check that.
Pertaining to the sort icon overlapping issue, its still there on the latest version of the webform module. So I'm changing the issue metadata accordingly. If you derive any other findings feel free to change accordingly!
Also attaching screenshots for the reference of my findings.
Hello there, Fixed the linting errors of the pipeline and hence the merge errors gets over now. So moving the ticket to "Needs Review" state.
https://git.drupalcode.org/project/drupal/-/merge_requests/7499#note_297751
sourojeetpaul → made their first commit to this issue’s fork.
Hello @Sandeep, I've successfully reproduced the issue you were talking about, and the patch provided by you also got applied successfully. After applying the patch the issue is resolved now and there is a breathing space between the section title and the close button. So moving it to RTBC++.
Attaching screenshots before and after applying the patch for reference!
I've resolved the issue that I mentioned on the previous comment, also observed that its a common behaviour for the primary tabs local task that's why it was affecting all the tabs, including the login page tabs. So in my commit I've addressed it globally which mitigates the issue. Please have a look:
https://git.drupalcode.org/project/business/-/merge_requests/17
Attaching screenshots for reference!
Hello, @Neha Wadhwani, I cannot find any style breaking issue on the content view page, I think what you meant here is the field labels coming in the full view mode which can be easily mitigated as correctly suggested by @Ashutosh from the manage display setting. But while trying to reproduce this issue I've observed rather another issue which caught my attention, is that the tabs are aligned vertically which is a bit weird, I think it'll be better if its aligned horizontally, and should only appear vertically through a hamburger menu menu on smaller screens. So I'm fixing that within this issue itself!
Attaching a screenshot for reference!
Hello, @adarshv I've applied MR !3 and the issue is resolved now, menu items are properly center aligned. Also the code changes look fine to me. So moving it to RTBC++
Attaching screenshots before and after applying patch for reference!
Hello @Ashutosh, I got a bit confused as I can see only two color variables are defined in the SCSS file and designed it accordingly.
Now not sure what kind of colour scheme you want for the tabs, I've prepared two different variants though, by aligning it with the home page colour scheme. Can you please confirm which one you would like to be incorporated here, then accordingly I'll update the MR.
Also I'm afraid then the submit button colour also needs to be changed accordingly as it looks a bit inconsistent to me!
Hello @Ashutosh Ahirwal, I've implemented the login page nav tabs and also addressed a few style breaks on the login page. Raised MR !1 for the same, please have a look:
https://git.drupalcode.org/project/business_bee/-/merge_requests/1
This is how the login page looks now: