- ๐บ๐ธUnited States mikelutz Michigan, USA
NW for Phenaproxima's comments.
- ๐จ๐ญSwitzerland znerol
Added tests for the new
UriHost
constraint. Note thatfilter_var($value, \FILTER_VALIDATE_DOMAIN, \FILTER_FLAG_HOSTNAME)
accepts every valid IPv4, thus removed the explicit call tofilter_var($value, \FILTER_VALIDATE_IP, \FILTER_FLAG_IPV4)
. Automatically closed - issue fixed for 2 weeks with no activity.
- ๐จ๐ญSwitzerland znerol
Gist of a slack conversation with @penyaskito:
@znerol saw that, but still think hostname should re-use symfony validator instead of just validating no control characters. The stricter the better. And even if we don't, I'm pretty sure we will need to reuse the symfony constraints at some point.
Turned out that the
Hostname
validator isn't quite enough, since thehost
parameter also should accept IP addresses. I've investigated whether it would be possible to implement this using theAtLeastOneOf
and a combination ofHostname
andIp
. But that doesn't allow for IPv6 literals (enclosed in brackets).I guess it would be best to just introduce an
UriHost
constraint, which implements RFC 3986 section 3.2.2. - ๐ฉ๐ชGermany rkoller Nรผrnberg, Germany
We discussed this issue at ๐ Drupal Usability Meeting 2024-05-17 Needs work . That issue will have a link to a recording of the meeting.
For the record, the attendees at the usability meeting were @AaronMchale, @benjifisher, @rkoller, @shaal, @skaught, and @simhohell.
If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.
While exploring the new responsive image form display widget we ran into one problem. On a standard 11.x-dev install with the responsive image module installed, and MR4628 applied, we had the following three options available for
Preview responsive image styles
:- Narrow
- Wide
Our assumption was that based on micro copy you would either get the preview image added in the dimensions of the chosen responsive image style or you have no preview depending on your choice. But problem was no matter what setting we set the resulting preview looked like this:
Our expectation, based on the micro copy, would have been that this would only be the result in case option 1, no preview, was chosen. So we were not sure if this is a bug, and in general we've asked us how this feature is intended to work? We would need some clarification in that regard before we are able to make any recommendations.
We will also continue our initial discussion we've started in regards of the micro copy. There was already a clear consensus to change the label
Preview responsive image style
toResponsive image style for preview
. "Technically" a user isn't "previewing a responsive image style", instead a preview image based on that responsive image style is being generated. About the corresponding description, there was the consensus to clarify where the image is shown (aka that it is shown on the edit form), and that "preview responsive image" is too verbose as well as misleading (There is only a single image being used, based on the responsive image style - but again we need to know how the feature is supposed to work).
One radio button option is calledBar with progress meter
while within the description it is referred to as aprogress bar
. Aside this inconsistency in terminology it was also noted that the first sentence of the description is imprecise, the throbber actually is communicating the status of the upload, it is telling the user if the upload is still underway or was already finished. But the microcopy about the progress indicator would apply to regular image widgets as well. So making adjustments in that regard would be more suitable for a followup issue to keep things consistent between image and responsive image widgets.But we will finish the discussions about the micro copy as well as the functionality in general as soon as the feedback is in. Thank you in advance!
Not sure what the best status would be in this case? I went with needs work but to postpone with maintainer needs more infos felt wrong since i am not the maintainer nor person working on this issue. I'll go with needs work, but please adjust accordingly if that is not the right choice and there is a more suitable one. Apologies in that case.
Automatically closed - issue fixed for 2 weeks with no activity.
Automatically closed - issue fixed for 2 weeks with no activity.
- ๐จ๐ญSwitzerland znerol
I added the
FullyValidatable
constraint to the @system.mail@ config. But honestly, I'm not quite sure on which level this should be added (config level or core datatype level, or even/additionally in @mailer_dsn.options.x@). - ๐ซ๐ทFrance andypost
Left review on MR - we can add proper types with deprecation message as 11.x going beta
- @narendrar opened merge request.
- Issue created by @narendraR
- @kunalsachdev opened merge request.
- ๐บ๐ธUnited States smustgrave
So applied the MR locally.
Update hook ran just fine
I'm still able to update the settings without issue
The proposed schema changes believe make sense.
+1 from me.
- ๐บ๐ธUnited States smustgrave
Left a comment on the MR.
Don't think we will need an upgrade path as the types don't seem to be changing but not 100% on that.
- ๐บ๐ธUnited States carsoncho
What I think is remaining is the tests updates. I see there's a few failing due to configuration tests as it's noting that the
core.entity_view_mode.*
config files have been changed and updated. This is to be expected given thehook_post_update_NAME()
being included here.Drupal\FunctionalTests\Installer\InstallerExistingConfigMultilingualTest::testConfigSync Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'create' => [] 'update' => Array ( - 0 => 'system.mail' + 0 => 'core.entity_view_mode.node.teaser' + 1 => 'core.entity_view_mode.node.se...result' + 2 => 'core.entity_view_mode.node.se..._index' + 3 => 'core.entity_view_mode.node.rss' + 4 => 'core.entity_view_mode.node.full' + 5 => 'system.mail' + 6 => 'core.entity_view_mode.user.full' + 7 => 'core.entity_view_mode.user.compact' )
Is the right thing to do here update the tests so they expect all those view mode configuration changes? Testing is an area I'd like to be able to contribute to more so any information folks can provide is much appreciated.
- Issue created by @kunal.sachdev
- First commit to issue fork.
- ๐จ๐ญSwitzerland znerol
Oops, did some work in parallel on
system.mail.mailer_dsn
over in ๐ Tighten config validation schema of system.mail mailer_dsn Postponed . - ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
Gave it a try and I think I ended up in the same place I ended up on Wim's computer.
Created https://git.drupalcode.org/issue/drupal-3440975/-/merge_requests/1/diffs for not polluting the original MR. I got stuck when in some place we are expecting typed data instead of the underlying value (or the opposite). Might be easier if we just extend from the symfony validators (as Regex does already), but definitely not as clean :-/
- ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
Discussed with Wim and we were looking at https://symfony.com/doc/current/reference/constraints/AtLeastOneOf.html, as hostname could be a hostname, a ipv4 or a ipv6. After some pairing we figured out that using composite constraints might be complex and could be deferred to a follow-up, as it's not very common to use a IP for a mail server hostname.
However, back at home just discovered the Symfony "Hostname" validator will consider "default" as an invalid one, and that's our default for e.g. when using sendmail, so we really need to figure this out here.