Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
This would be more useful if the message displayed when a user reached the session limit was configurable. Providing a patch.
Comments
Comment #2
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedComment #3
nkoporecHey tested your module, everything seems to be working.It only has one coding standard issue, in settings form you should use $this->t(), instead of t().
Comment #4
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedGood catch. I think I have that updated here.
Comment #5
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedComment #6
nkoporecHey, the patch doesn't apply on the 8.x-1.x branch(using latest drupal 8.5) ... when checking the patch it throws this error:
Comment #7
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedAh, my bad! I kinda did a rush find replace on that one. This patch applies cleanly.
Comment #8
nkoporecHi @mikeegoulding, tested the latest patch and applies cleanly.Good job!
Comment #9
kristiaanvandeneyndeSeveral files have been stripped from their ending newline. Please check your IDE and make sure that it always adds a newline at the end of a file.
Remove this empty line at the start of the function and between the function declaration and docblock.
Not sure you should use t() here. The settings Yaml file requires the string to be specified in the default language (which is often EN)
Please properly inject this service.
You even have a @todo for it, so please get rid of these and properly inject them.
Comment #10
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedAlright. I think I've made these changes. I was originally trying to not make any major changes to other sections. Should have done the dependency injection from the start.
Comment #11
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedComment #12
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedAnd I just realized my patch is blank and there are new commits on this that add some of the dependency injection mentioned in previous comments. I'll have to fix this up later.
Comment #13
mikeegoulding CreditAttribution: mikeegoulding at Ashday Interactive Systems commentedRe rolling patch against latest 8.1-1.x branch
Comment #14
avinash_odal CreditAttribution: avinash_odal commentedThe Session Limits does not work any more, tested on Drupal 8-3-5 and Drupal 8-5-3. It was working with Version 8-2-x versions of drupal, however i tested with the 8-3-x branch and the 8-5-x branch and the session limit doesn't work.
Comment #15
mike.vindicate CreditAttribution: mike.vindicate commentedMade the patch apply to the latest version.
Comment #16
idebr CreditAttribution: idebr at iO commentedtype: string
is not translatable. Let's usetype: label
so the value can be translated through Configuration translation. Documentation about these data types is available at https://www.drupal.org/docs/8/api/configuration-api/configuration-schema...Comment #17
mike.vindicate CreditAttribution: mike.vindicate commentedChanged type to label.
@idebr thanks for pointing it out.
Comment #18
mike.vindicate CreditAttribution: mike.vindicate at iO commentedAttached patch which can be applied to the latest version 2.0.0-beta2.
Comment #19
mike.vindicate CreditAttribution: mike.vindicate at iO commentedSomething went wrong with the creation of the patch in #18, missing the settings form.
Attached new patch with the added form element and saving of the value.
Comment #20
Ambient.ImpactWould it be possible to make the text content filtered text? For example, if I want to add a link to a contact form in the message, this is not possible with the patch in #19 as it just outputs it as plain text.
Comment #21
Sophie.SKThis should probably be rolled against the latest version (2.x-dev) rather than the 8.x-dev branch, though this patch does apply to the latest 2.0 beta too.
It would probably be good to have the default text in the form too. If an admin doesn't make a change but saves the form, then it shouldn't override in config. I'll make some changes to the patch.Noticed that it's there in the install config, but I already had the module installed, and hadn't run updates. Doh!
Comment #22
Sophie.SKHm, having spent a while trying to figure it out, I can't get the formatting to work right on text output. Sorry - going to mark this as needs work but against the current version.
Comment #23
yasmeensalah CreditAttribution: yasmeensalah at Vardot commentedRe-roll the patch to make it work with this patch https://www.drupal.org/i/2985067