did you disable css/js aggregation or clear the cache? it's on by default with umami
The no/js version used to interfere with the navigation on the website but it seems ok now, priority normal is fair.
I suggested the @ to keep some level of gitlab integration when not using the name + email pattern.
If it gets more in the way than a "simple" By: d.o_username let's just go with that and not have any special integration with gitlab on the commit message viewing (I'm sure we could find a way to make a gitlab plugin to do it if that's really important)
- Wrapping styles in the extra specificity selector to force the style to be applied: OK
- extra-specificity-hack/extra-specificity-trick: extra-specificity-hack. We have a long history of CSS using "hack" to find workaround for browser support
I hear the concerns, so what should we do? name + email is a no-go per previous discussions.
What would be useful: d.o usernames without @, actual d.o URL to the user, a link to the credit record?
Problem with stying, the rules order changed and messed up the table headers style.
Committed and pushed d5dcae6c048 to 11.x and b187cc5e743 to 11.3.x. Thanks!
There was a new version like an hour ago, but let's get using this plugin for now it's up to date enough for our purpose.
Committed and pushed 703e5d41895 to 11.x and 256c05ea65a to 11.3.x. Thanks!
Committed and pushed e19de46381d to 11.x and aeae5fd1c0e to 11.3.x. Thanks!
I think task is valid, fix carry the meaning that something is broken. Some follow-ups are not fixes, features, or chores. Like adding additional tests for a feature would qualify as a task for me
Got an assist from @latent with the code: https://gist.github.com/MichaelWest22/7ca5b715b368ec1289352d4eeb310182
This moves the code to an htmx extension, and we might be able to use an extension to declare new swap styles to manage modals and messages.
worked out the problems to get things working with that new branch. This is now behaving like our current ajax framework. Assets needs to be loaded before DOM replacement happens.
Committed and pushed 2592e8ba422 to 11.x and ea0226fd568 to 11.3.x. Thanks!
Committed and pushed f7e7f3efe42 to 11.x and c4340b27c61 to 11.3.x. Thanks!
haha yeah views is a good way to know if we broke the JS usually.
We already take over jquery ui _focusTabbable to make it use focusable library: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/dialog/d... how is this MR different?
Let's go with that then. We're reformating the informations we already have.
And we can discuss the trailer format once we use the gitlab ui to do the commits.
nod_ → changed the visibility of the branch 3543076-simplify-agreed-commit to hidden.
Correct, and the discussion did not take into account that the goal of Co-authored-by is to either compensate for a lack of proper crediting tools or when commits logs are the whole crediting tool, and neither applies to us.
also where did the brackets come from around the nid? that's something you'd use for jira-style ticket names since there are no defined delimiter, which is not our case.
Removing the brackets, not using co-authored-by because we have a proper credit system in place for that kind of informations and not to confuse tools that might rely on an email being present, we're left with:
feat: #123 something something
By: @nod_
By: @dww
By: @cmlara
We're all happy with that?
Concerned were raised in ✨ Simplify agreed commit format Active about the fact that we're not following conventional commits spec and is proposing to move the issue ID after the commit type information.
nice example in #61, thanks.
To avoid going in circles I'd like to point out that Co-authored-by is not a conventional commit spec/requirement. It's a practice started by github/gitlab to compensate the lack of attribution tools in their system (we tried to help them get better tools, it didn't work out). On our end we have very powerful attribution tooling so the technicalities of how Co-authored-by works are not a concern for us, since this is not how we give attribution, or even show how someone contributes to the community, that happens on the d.o user page, not the gitlab interface. To me the commit message is to help when I do a git blame, not much more.
If there are scripts that expect Co-authored-by to have an email, then we don't use that and use "By" as we've been using for 2 decades.
We used to have
Issue #123 by nod_, dww, cmlara: something something
, and very quickly we could have:
feat: #123 something something
By: @nod_
By: @dww
By: @cmlara
Same exact user information just presented differently. Can we get that resolved and move on?
My main objective with htmx and forms is to make sure htmx and regular requests follow the same code path, unlike what's happening with Ajax today. I'm thinking most uses of htmx will be in the context of a form (for now at least) where it makes sense to add the wrapping format by default. Even if we query an htmx route, adding the wrapper format parameter shouldn't be a problem.
All that to say I think we should keep the default as adding the wrapper format.
+1 to the simplification but it seems like we're loosing some informations as to why it's not necessary when using ckeditor. Can we get more of the comment ported to media in addition to the logic?
for the tooling litteraly any tool saying they support conventional commits would work. We talked about using well formatted commits to generate changelogs and such. I would expect some tests with anything in here https://www.conventionalcommits.org/en/about/ and maybe more specifically tools that claim to help generate changelogs like this one: https://github.com/marcocesarato/php-conventional-changelog
might not be used in core but could be used in contrib.
Change record should explain a bit where that is used (not used in core yet) administrative uis of drupal canvas and display builder for examples.
Also SDC do not stand for 'Structured Data Component (SDC)', so some work to the change record wouldn't be bad
reverted original issue. I'll let you decide if we keep this one open to fix the underlying issue of alignment here or in the original issue
I'm going to need people to actually try some tools and report back here with the results before we talk about re-decide anything regarding that first line commit text.
I found unexpected behaviors testing the usernames in #46 so let's not assume what would or wouldn't work with existing tooling. I'd try all the variations (for the first line) of 🌱 [policy] Decide on format of commit message Active and see what actually comes out.
Main reason was to get the contributors out of the first line, so any variation would achieve this. Now if the rest of contrib moves to this format, let's make sure the tooling they'd be likely to use gives something useful
Thanks for finding the history on this one
Committed cff1082 and pushed to 11.x. Thanks!
We need a 11.2.x version of the MR since it doesn't cherry pick cleanly
I don't think this warrant a change record, i won't publish it.
Committed 4f4ff21 and pushed to 11.x. Thanks!
small conflict to resolve in core/profiles/demo_umami/tests/src/FunctionalJavascript/AssetAggregationAcrossPagesTest.php
Committed and pushed adddba60e2b to 11.x and d5fe4c8715d to 11.2.x. Thanks!
MR looks good, a bit worried about adding more drupal specific logic to the attributes object, and more drupalism to SDC but that's a tradeoff that looks worth it
seems like the last commit undid a bunch of things on the MR (including the fix to make sure we don't have "- Select -" in the push url).
reapplied the fixes and used the new method to get the trigger name.
Hey a HTMX issue I can commit, yay!
Committed 5edaa75 and pushed to 11.x. Thanks!
Committed and pushed 09656a796c7 to 11.x and e0f3aad5bab to 11.2.x and 3c7cc933c02 to 10.5.x and 4f88d508615 to 10.6.x. Thanks!
Tried it out and that is the appropriate fix. Even if it's not in a noscript element it just makes sense to check if the element exists before trying to run things off it.
About #34 and #39, adding a @ in front of the user name would link to the user in the gitlab UI: https://git.drupalcode.org/project/drupal/-/commit/6ba27a8298b3bdf05bab4...
[#122312] feat: something something
By: @nod_
Would transform to a link to my user when reading the commit message, just like in MR comments. I tested the standard format and pictures are broken for some reasons as you can see here, and what gitlab links to is a mailto link, not even the user profile: https://git.drupalcode.org/issue/drupal-3548100/-/commit/4ef708e0473351b...
So either we keep it as bare username or prefix them with @. The actual standard format doesn't look very helpful for us.
started to go another way in 📌 Ajaxify the user interface translation forms Active
Instead of handling that in the backend, the query string is added from the front and we have an extra drupal-specific attribute that controls it
I think I found the sweet spot for using/cleaning the extra parameters in 📌 Ajaxify the user interface translation forms Active . If wrapper format is not a problem we still have to manage ajax_page_state.
Also if we avoid dedicated endpoints for views, we should be able to simplify the backend too.
I agree that using _format is a bit of a stretch so I don't mind a dedicated key.
I'm leaning towards using boost with a drupal-specific attribute to control the wrapper format, ajax page state and push url.
Seems like it! the boost attribute does more than I need though,
Checked it out more closely, we kind of have nested attachBehaviors call, that isn't idea.
Instead of that we can just try to call the function "safely". So doing
$(document).on('drupalContextualLinkAdded', (event, data) => {
Drupal.ajax?.bindAjaxLinks(data.$el[0]);
});
and keeping the backend code that conditionaly load core/drupal.ajax should be enough
Oh nice, and the perf test looks great with 27k of JS removed :D
Tests are green, since we never used fetch directly in core before the test framework didn't know how to wait on it.
We should be able to find a way to make the ajax contextual link not a dependency. Does anyone has a example of this being of use? was it for quickedit back in the day maybe?
This MR builds on 📌 Improve url management of the Htmx object Needs work , 📌 [PP-1] Refactor ConfigSingleExportForm to use HTMX Postponed , 📌 Ajaxify the user interface translation forms Active , which is why the diff is important.
To test, toggle the "use ajax" setting on the view you want (I only tested the /admin/content table and exposed filter at this point).
Instead of a custom _htmx_route, maybe we can use _format: drupal_htmx or _format:htmx in the route definition.
See how lupus_renderer handles that, creating a new _format:custom_elements for api responses.
The new tests are still passing and it makes sense based on upstream code,
updated the version in the change record, it'll be 11.3
Committed c8dae79 and pushed to 11.x. Thanks!
I don't think that warrant a change record, it will just work™ now
Committed 103108a and pushed to 11.x. Thanks!
should be good to go, maybe comments needs some work? not good at that.
Played with the patch a bit, the attribute stuff I'm doing with htmx doesn't break. And things seems in order in the admin UI. I asked @grimreaper to test it with ui_patterns/display_builder since they might do more complex things with twig than core. Maybe we need to test twig_tweak too?
got the pagination working but it's not limited to this form, I don't know what else it might break
Got it sort of working but there are problems:
- Pager links are very broken, they get all the form stuff added to the query string and since we exclude the libraries trying to navigate to them is just not working.
- The session values never gets updated so default filtering stays at what it was. When the page is refreshed we don't get the last filtered thing.
We should be able to deal with it here, need to try a few things where we might have to refactor the backend a bit
Now that we have htmx in core it could be a bit easier to manage that. We don't use htmx for exposed forms but that should be something to look into
Sorry late to the follow-up. Since we have different rules between 10.x and 11.x I'd rather avoid the inconsistency and not backport this.
Thanks for the dedicated MR though!
not backporting to 10.x
Committed and pushed 3a9f2bab7e5 to 11.x and 17f7df3e200 to 11.2.x. Thanks!