πŸ‡ΊπŸ‡ΈUnited States @olarin

Account created on 12 July 2008, over 16 years ago
  • Software Developer at KosadaΒ 
#

Recent comments

πŸ‡ΊπŸ‡ΈUnited States olarin

Hm, I think so. That ticket made the text configurable, and also made the default text a little more generic, rather than assuming (perhaps incorrectly) what went wrong, so it addresses the biggest concerns of this ticket, preventing a reasonable configuration (only one shipping option enabled) from resulting in misleading errors out of the box. If there was more we could do to differentiate between different problems and make it possible to give more specific, helpful error messages, that would be nice, but as I alluded to previously that's probably not possible unless shipping providers give us additional data as to why they returned no valid shipping options ("I can't ship to that address via these methods" vs. "I don't recognize that as a valid address").

πŸ‡ΊπŸ‡ΈUnited States olarin

FYI, I had a client encounter this same issue and get confused by a page display on a View with a contextual filter in the URL (to the end user it looks like multiple distinct pages, one for each valid value of the contextual filter, whereas it's obviously just one route underneath). It might be nice if there was a way to at least describe the problem more clearly to non-technical users in the error message, but I'm not sure what that would look like.

πŸ‡ΊπŸ‡ΈUnited States olarin

Fair point. I was looking at the OOP code and overthinking it and forgot about the good old standby option of a form_alter hook. Thanks for the reminder.

On further reflection, rather than just closing this, I'll change this to a low priority bug report rather than a feature request, since given that the module hides the radio buttons in this particular use case, instructing the user to make a selection they can't make is unhelpful. (Come to think of it this situation might also occur if you had multiple shipping options but none of them were valid for the address you entered, either because it was an invalid address or simply not covered by the available options.) There's only so much Commerce Shipping can do at this level to know what went wrong (for instance it can't necessarily discern why a shipping provider didn't respond with a useful quote for a particular request), but it should be able to at least differentiate between "the user didn't select an option" and "we couldn't give the user any options to select" and attempt to provide some slightly more helpful guidance in the latter case.

πŸ‡ΊπŸ‡ΈUnited States olarin

Yes, the latest version of the pull request seems to work properly for me, thanks.

πŸ‡ΊπŸ‡ΈUnited States olarin

Tried both the merge request and the patch in #4 (not at the same time of course). In either case, while I could successfully edit the text format again, when looking at the Markdown filter settings in the text format, the following error was displayed:

Error message
Drupal\markdown\Form\ParserConfigurationForm::buildParser(): Argument #2 ($form_state) must be of type Drupal\markdown\Form\SubformStateInterface, Drupal\Core\Form\SubformState given, called in /var/www/html/webroot/modules/markdown/src/Form/ParserConfigurationForm.php on line 239

Furthermore, if I save the text format (even without making changes), then it seems to break formatting entirely, i.e. a text field using Markdown is displayed without _any_ formatting, as in no HTML tags at all, not even line breaks or paragraphs. (Fortunately I tested this on a dev copy of a site and not a live site.) There are other filters at play on the site in question, but I tried turning all the rest of them off and confirmed that Markdown specifically is the one causing the formatting to be stripped (whereas if I turn Markdown off the text at least gets paragraphs and line breaks again, and the other filters can work as expected).

πŸ‡ΊπŸ‡ΈUnited States olarin

This appears to be a guaranteed issue for any new site audacious enough to attempt to use any install profile other than standard (including the core-provided minimal), and there's no workaround to my knowledge, so I think it would qualify as Major priority.

I added a MR for the 10.1.x branch. I'm not sure where to start on adding a relevant test.

πŸ‡ΊπŸ‡ΈUnited States olarin

Olarin β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States olarin

PR created. I also fixed the outdated reference to "terms" (rather than entities) in the autocreate option description while I was at it.

Production build 0.71.5 2024