Finland
Account created on 20 December 2010, over 13 years ago
  • Staff Software Engineer in the Drupal Acceleration Team at Acquia 
#

Merge Requests

More

Recent comments

🇫🇮Finland lauriii Finland

📌 What component modeling approaches are supported Active describes several ways users would construct components in the Experience Builder context. 📌 Create an example set of SDC components Active starts implementing example components using these patterns to identify any gaps in the SDC system to express these patterns.

🇫🇮Finland lauriii Finland

There's some great work on 📌 Create an example set of SDC components Active to draft few components to begin with.

🇫🇮Finland lauriii Finland

The difference between Recipes and Experience Builder is that Recipes was targeted for core from the get got, whereas Experience Builder may be in contrib for the foreseeable future. Is this something we'd do only until we can release an alpha release or would this be an approach we'd take for longer term? If this is something we are planning for longer term, we should consider what's the impact for end-users as well.

🇫🇮Finland lauriii Finland

Committed c27cc9b and pushed to 11.x. Also cherry-picked to 11.0.x, 10.4.x and 10.3.x. Thanks!

🇫🇮Finland lauriii Finland

Navigation beta experimental

🇫🇮Finland lauriii Finland

Looks like we have sign-offs from all roles and 📌 Make libraries with JS internal, at least for beta Active has been committed.

🇫🇮Finland lauriii Finland

It doesn't look like Gin is integrating with the Navigation through the libraries so it should be fine. I tested Gin manually with the patch and I didn't find any regressions.

🇫🇮Finland lauriii Finland

@e0ipso are you saying that in your mind, the syntax proposed in #26 leads into a better DX for a user who is a) not familiar with Drupal b) not a senior developer c) and hence does not know JSON-Schema? Is it because it's able to better solve the problems you are raising in #30?

🇫🇮Finland lauriii Finland

Creating components is a common enough task that it should be really easy for someone who is a) not familiar with Drupal b) not a senior developer and hence c) does not know JSON-Schema. This is highly relevant for workflows where the user has a pre-existing design system that they want to integrate with Drupal. For this persona, type: 'drupal-image'would be the north star experience because they don't care whether we use JSON-Schema or not.

That said, using JSON-Schema as a starting point makes sense, but we should not let it define what is the developer experience we provide. We just don't want to require users to learn JSON-Schema in order to be successful. Because of this, we need to consider how we can simplify some of the more complex workflows instead of being dogmatic about implementing the specification.

What's being proposed in #26 seems to be making this more reasonable already. However, even that could be seen as a strange by someone who is not familiar with the syntax. This user might ask; what is difference between $ref and type, why do I now need these two now, and why does the value need to follow a strange pattern with ://.

🇫🇮Finland lauriii Finland

Committed 90d6e3b and pushed to 11.x. Thanks!

🇫🇮Finland lauriii Finland

Committed 0466b81 and pushed to 11.x. Also cherry-picked to 10.3.x. Thanks!

🇫🇮Finland lauriii Finland

lauriii changed the visibility of the branch 3225621-use-media-query to hidden.

🇫🇮Finland lauriii Finland

lauriii changed the visibility of the branch 3225621-9.4.x to hidden.

🇫🇮Finland lauriii Finland

lauriii changed the visibility of the branch 3225621-11.x to hidden.

🇫🇮Finland lauriii Finland

lauriii changed the visibility of the branch 3225621-10.0.x to hidden.

🇫🇮Finland lauriii Finland

I'm not sure I agree we should show contextual link for the navigation. The feature to customize the navigation is great for power users, but it seems unlikely that most users would customize the menu. Basically as @KeyboardCowboy is saying, even for power users it's something you'd once and then never do it again. Because of this, I think the contextual link is adding extra noise that is not warranted. We already hide contextual links elsewhere in the admin UI for similar reasons.

🇫🇮Finland lauriii Finland

Adding 📌 Mark Navigation as beta experimental Active as a reference for beta stability sign-offs.

🇫🇮Finland lauriii Finland

I opened 📌 Mark Navigation as beta experimental Active to decide on beta stability, so that we can commit the module as alpha before that. I noticed that there are some comments missing, and other minor things that could be improved. I don't think these need to block the commit because it would be much easier to fix these outside of this massive MR, especially given that the contrib module is now out of sync.

Committed dae3853 and pushed to 11.x. Congrats everyone! 🎉

🇫🇮Finland lauriii Finland

Let's make sure that any caching strategy that we implement is built in mind that we want to keep the navigation completely static in between page loads with the exception of the active marker moving to a different menu link. This means we want to avoid any flickering or even loading animations when you navigate within Drupal, including the username. The navigation is really close to being able to achieve this which is quite remarkable.

🇫🇮Finland lauriii Finland

I'm wondering if we should introduce the textarea as an advanced option on top of the current option guided option so that you could do a bulk update multiple values easily, as well as copy and paste a list from a text editor. What I'm thinking is adding a link on top of the table with a text "Bulk edit", which would open a modal dialog with the values in a single textarea. Once you submit the form on the dialog, it will automatically populate the table based on the values you've given in the dialog. This way we would probably get best of both worlds.

🇫🇮Finland lauriii Finland

The high level idea and plan, as well as the implementation plan for the navigation seem fine. I also already tested the module myself and it seems to be pretty far along. It's been also great to see so much activity around this in the issue queue and Slack. 🤩

P.S. I moved 📌 [META] Layout redesign Active to the "Next" section since there aren't enough details to approve the plans for that yet. Once we have defined a plan for that, feel free to move this issue back to "Proposed Plan" state to request a review from product managers.

🇫🇮Finland lauriii Finland

I tried clearing caches and it works perfectly now. 💯 Great work on this and sorry about the noise 🤩

🇫🇮Finland lauriii Finland

I agree that the current solution may not be perfect but it solves the described problem and is better than what exists today. So even if this is not perfect, it's good enough. Based on that +1 for moving forward with the proposed solution.

We can validate this future in future by testing how this performs when we do UX testing next time. We can decide if this needs an additional iteration based on that.

🇫🇮Finland lauriii Finland

We should also update the local development instructions in https://www.drupal.org/docs/getting-started/installing-drupal . I was testing if I could get to the DDEV documentation from the front page without using search. This was the first page I landed on because it was linked in https://www.drupal.org/project/drupal/releases/10.2.4 .

🇫🇮Finland lauriii Finland

An additional scenario that this would help with is when people upgrade contrib without updating all modules. At least with this, they would get a warning that the module is incompatible with the core release. This could be happening in cases where modules have been pinned to a specific release for example due to a patch that applies on top of the specific version.

🇫🇮Finland lauriii Finland

I agree that the deprecation approach is a bit strange because it's fixing something to break it later. It tried to optimize to minimize the pain, and I don't think the cost for that was too high. That said, I don't have a strong preference to any particular direction so long as we get the bug fixed. 😇

🇫🇮Finland lauriii Finland

FWIW, the latest commit looks good 👍

🇫🇮Finland lauriii Finland

Feedback from @Wim Leers has been addressed 👍

🇫🇮Finland lauriii Finland

The solution looks, although I realized there isn't a perfect solution available to this because people may still continue using render arrays for #title. I think what's here is fine for now and we can revisit this later in case that we find a dialog solution that supports markup for the title.

🇫🇮Finland lauriii Finland

The aspect that required security review has been addressed. I think a regular committer review is sufficient now.

🇫🇮Finland lauriii Finland

The challenge with #247 is that there's no such state as nothing required. When nothing is required, if you enter end date, the start date would still be required. I think the current proposal is good enough, and that's why I think we should move forward with it. We can implement improvements on top of this in future.

🇫🇮Finland lauriii Finland

The rebase seems good. 👍 Thank you @kunal.sachdev!

🇫🇮Finland lauriii Finland

I discussed with @alexpott about the failing test. The performance test is failing because this is introducing an additional cache query. We discussed about potentially adding static caching but agreed that the additional cache query was fine. So basically what we could do, is bump up the query counts by one.

🇫🇮Finland lauriii Finland

+1 for the fix. Good job figuring that out. 👍

🇫🇮Finland lauriii Finland

Looks like core/modules/filter/filter.filter_html.admin.js has dependency on it, but it's not declaring it's dependencies correctly... 😅

🇫🇮Finland lauriii Finland

It doesn't look like the MR is implementing the feedback from #97.

🇫🇮Finland lauriii Finland

Maybe we should consider opening a follow-up for some next steps. I think the first next step would be to always add the new menu item to the end of the list. Another next step would be trying to come up with a better UI for users to select where the new menu link should be added.

I think this also needs a change record.

🇫🇮Finland lauriii Finland

Looks like the bug is fixed and #8 has been addressed. Would be nice for sure to have a common component between these two use cases but it doesn't have to happen here.

Committed 89fde7a and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

🇫🇮Finland lauriii Finland

We opened #3391592: Define goverance changes that the core committers are responsibile for for discussing how the committers make changes to the governance. I reverted the governance row change from the PACSI matrix to address #84. This also makes it consistent with the "How are decisions made?" list which states the following:

If the change affects the governance, philosophy, goals, principles, or nature of the project, it must be approved by the Project Lead.

🇫🇮Finland lauriii Finland

For me the bigger issue is that this ends up happening so late in the process, and only when some kind of release deadline was approaching - i.e. pretty much every minor release, someone whether a core committer or other contributor will start pinging me about some kind of release or framework manager sign-off I often hadn't heard about before.

My own experience has been that generally people really start pushing for sign-offs when there's already risk of it being too late for the decision to happen. I've suffered from this too. I remember thinking that the committers probably have more important challenges to concern themselves with. To address this, we need more proactive involvement from the committers to enable decision making early in the process. We need a culture where it's clear that the committers want to be involved and that reaching out to them with a low bar early on is not just allowed, but expected.

On top of that, I think it would be helpful to have a discussion on how different roles would like to sequence decisions, e.g. at which stage of the process do we start becoming uncomfortable to mark a module stable. I don't know that we should set hard limits but it would be helpful to hear what these timelines look like because now the only timeline we track is the actual deadline.

I'm comfortable with Product Managers being the tie breaker in the case of a stalemate but it would be good if their thought process/research/reasoning was made public. With any luck that will change the stalemate and prevent needing to use a tie-break option.

I believe product managers should conduct their work transparently, and give other roles an equal opportunity to be heard. I believe the current change already documents these both: "While product managers lead the decision-making, their approach should be transparent and include everyone's input.". Do you think that sufficiently covers it, or do we need to make some changes to that?

I don't mind if product managers get the final casting vote when there is a deadlock, but this leaves open the possibility of product managers overruling all (or even most of) the framework and release managers about a particular decision, which in such a case would make release and framework management impossible to carry out.

In practice, it would not be easy for product managers to make a decision where all of the framework and release managers are against the decision. Many of our decision require commitment from all of the roles, and this is hard to achieve in a situation where everyone in those roles is against the decision. For this reason, this would not be effective, even if the governance technically allowed it.

Thinking about that example a bit more, it seems that a situation where the release managers and framework managers are against product managers would likely point to a bigger problem. In this situation, it seems more appropriate to have discussion about the vision and the strategy. In particular if the vision and the strategy are clear enough, if there's alignment on them, and if the team is committed to executing them.

To me this seems more relevant to scenarios where there are some people vetoing a decision, but there's at least some support for the decision from all roles. These are the scenarios where this could help us avoid postponed and diluted decisions.

🇫🇮Finland lauriii Finland

I am moving the issue back to NW for Comment #13. Losing the notice about media and image fields is a regression. I am adding a note to the issue summary so that we do not lose track of this regression.

I'm not 100% sure the message is needed for the media type at least when not editing fields for media entity. Maybe on this issue we move the message to the field config edit form and discuss in a follow-up if we should not display it when editing fields for other entity types than media.

🇫🇮Finland lauriii Finland

Updating the proposal based on the latest discussions on the issue. I'm removing the "needs product manager review", but I'm adding the "needs framework manager" review.

🇫🇮Finland lauriii Finland

Committed 77888cf and pushed to 11.x. Also cherry-picked to 10.2.x as a non-disruptive bug fix. Thanks!

🇫🇮Finland lauriii Finland

Committed 869929d and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

🇫🇮Finland lauriii Finland

Added a comment in the MR

🇫🇮Finland lauriii Finland

xjm credited lauriii .

🇫🇮Finland lauriii Finland

Thank you @frankdesign and @mrshowerman! Based on #164 and #165 +1 on the feature from me. I haven't looked at the actual code though.

🇫🇮Finland lauriii Finland

Committed e16d8e0 and pushed to 11.x. Cherry-picked to 10.2.x as a non-disruptive bug fix to Claro. Thanks!

🇫🇮Finland lauriii Finland

Based on the number of followers and participants, this seems like a feature that several folks would like to have. I'm not sure I understand when is this feature used and I didn't find any discussion about this from the thread. It would be helpful if folks could comment here / update the issue summary to explain what are the use cases for this feature. 😊

🇫🇮Finland lauriii Finland

Committed 1e8ae69 and pushed to 11.x. Cherry-picked to 10.2.x as a non-disruptive a11y bug fix. Thanks!

🇫🇮Finland lauriii Finland

I'm wondering if we should special case the default modes and allow creating that in case that it doesn't exist? It looks like it's not guaranteed that all entity types create the default view mode for all bundles. Node module is ensuring that the default modes are created in node_add_body_field.

🇫🇮Finland lauriii Finland

I got stressed at first because I thought I had somehow accidentally changed the button id in 📌 Update CKEditor 5 to 40.2.0 RTBC 😅 Luckily it looks like it was just a left-over change from an approach that I reverted there. This seems like a normal bug given that you can see what the button is by hovering it.

Committed 49175bf and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

🇫🇮Finland lauriii Finland

The overview pages are generated based on menus, so unless the module is doing something really unique (e.g. with preprocess), all of the links appearing in the overview pages should appear in the menu. See \Drupal\system\SystemManager::getAdminBlock if you're curious how the overview pages are generated.

However, section pages built using other mechanisms like local tasks (i.e. Block Layout) or tables is still a common pattern, so we need to have some level of support for getting to the parent link. For example here's the same problem with Paragraphs module:

Out of the box, Drupal doesn't provide links under the Block Layout link. I'm wondering which module is providing those links? 🤔

🇫🇮Finland lauriii Finland

I'm not sure why I was not seeing the setting being saved in my previous testing. I tested this both with existing fields and new fields and it seems that the value gets saved correctly with the MR. FWIW, it was working in the UI in all of my previous tests so maybe I was not looking at the right value with xdebug.

So for what I'm concerned, the MR should be ready. We can work on moving the settings to the storage part of the form in a follow-up issue.

🇫🇮Finland lauriii Finland

lauriii made their first commit to this issue’s fork.

🇫🇮Finland lauriii Finland

@johnpicozzi do you have a specific reason why you'd like to open "Structure" page? Wouldn't you be able to access all of the links directly from the navigation? I can definitely see that not being able to access "Block layout" is a problem.

🇫🇮Finland lauriii Finland

The MR should be ready to go. I opened a core issue for this problem 🐛 Field config edit form handles form state changes inconsistently Active but it probably makes sense to merge the fix here before the core issue gets fixed.

🇫🇮Finland lauriii Finland

lauriii created an issue.

Production build 0.67.2 2024