🇦🇺Australia @pameeela

Hmm, yeah, I agree. The reason it wasn't made optional is that we don't have a great way to provide the option to users within the one recipe, and I think having a separate recipe just for autocomplete would be somewhat odd.

First thing I guess it to determine whether it actually can be provided as an optional add-on, even if it's a separate recipe? In some cases, things can't be split because there is no way to manipulate the config as is necessary.

Ahh thank you!

I prefer to keep this in the theme over ECA or Block Class mainly because it is theme-specific, and it's a one-off. We want to avoid adding modules that will possibly conflict with XB. And although I love a good ECA fix, this feels pretty random. I don't think we want to add the class always, and if we end up just adding it for this theme then that seems like it should just be in the theme where it's easier to find.

Screencast of the scroll without the CSS fix.

@mherchel

had to add home-specific CSS for an extremely annoying bug. There was a horizontal scroll on smaller screens with this change because there's a width: 100% coming from another place that is overriding the width: 1px that is set in visually-hidden. So overrode that with a width: auto for this block only.

Do you have another suggestion for how to fix this? (Note that there's an issue with the MR, I'll fix the path -- I was testing it just in base.css and didn't properly update it).

Credited @heikkiy for catching the variable duplication and posting in Slack.

No reports of this since the launch of 1.0 so maybe obsolete?

Although this looks nice in the design, it didn't translate that well to the actual demo content. We ended up updating the content type display to use the name field in the card but the logo in the full page display, so they don't both appear together. They would generally be saying the same thing anyway I think.

Given that this theme is really for demo purposes only, I don't think we need to pursue this any further.

As noted in the IS, these changes would be made in Smart Date so moving it there.

Steps added

There's no change to the appearance, it just uses a smaller image for (oh-so-slightly) better performance.

LGTM

Thanks @dunx, I've gone through them and updated them variously. Several of them need more info (or for someone else who knows more about it to look at them!).

Looks like this applies specifically to the language drop down, which is no longer present.

Again not really sure how to fix this or honestly what it specifically refers to. Can anyone update with screenshots showing the issue and suggested fixes?

Not sure how to reproduce this. Not seeing any additional elements when tabbing, it goes straight from the password field to the finish button. If there is an additional focusable element that seems like a different bug, because I don't think there should be?

Not sure how to fix this one?

Updated to #8e929c for contrast of 3.11:1.

@phenaproxima annoying that we have really good test coverage of the meta tags but the front page requires separate config. Should we try to replicate this coverage for home? Or just quickly check for the values?

Either way would be good to get this fix in for 1.0.1 and follow up with tests.

@ultimike fair question!

We don't want to use the term 'Recipes' in the UI at all, but ended up with that one instance because of some late changes in PB that we were unable to override. It's a Drupal-specific term that is meaningful in terms of the technical implementation, but end users should/will not care what is under the hood. Rather than going all in on the term just because of this one instance, we decided to try to hold the line, even though it means some inconsistency in this first iteration.

Ideally in future we can group ALL add-ons to Drupal under one umbrella and users won't know whats a recipe vs a module (vs whatever else we come up with in the meantime). They're all just things that extend your site. The fewer Drupalisms users need to learn, the easier it is to adopt! Or so we hope...

This isn't about performance benchmarking, it was research on CMS features and journeys. It was crowdsourced and probably can be closed once credit is added.

pameeela → created an issue.

Ah, performance is 99 sometimes right after a cache clear :)

@mgifford I am getting all 100s locally with no changes other than 🐛 Front page meta description still references obsolete node:summary Active (as long as the SEO tools recipe is applied).
Setting a cache here /admin/config/development/performance

What exactly does this refer to? By default it's set to 15 min. Just looking for specific actionable tasks and I'm not sure what they are!

pameeela → created an issue.

Hey @anish.ir thanks for trying to take this on, but let's leave it postponed until we have confirmed the text.

This is just default Olivero, the same is true for any instances of .button, e.g. the 'Apply' button on the blog view.

The default color is:
--color-text-primary-medium: var(--color--primary-40);

And on hover:
--color-text-primary-loud: var(--color--primary-30);

So there's hardly any difference. I'd probably lean toward trying to fix this in Olivero?

Created an MR, had to add home-specific CSS for an extremely annoying bug. There was a horizontal scroll on smaller screens with this change because there's a width: 100% coming from another place that is overriding the width: 1px that is set in visually-hidden. So overrode that with a width: auto for this block only. But there may be a better way to fix this, I'm not sure.

Isn't the site name usually the h1 on the homepage?

Visibly? No, not usually. I can see that Drupal.org uses the visually-hidden class.

So yes we need to update the block config and preprocess the title on home to add this class.

OK, I understand the confusion now :) You are talking about the tags in the node teaser. But that's not the problem. Drupal CMS does not output the tags in the card view mode. The tags field that is getting flagged is the one on the full page display.

Here is a summary of the current state:

1. In core, the markup works for the teaser view mode (which has an h2 for the title)
2. In Drupal CMS, the markup for the card view mode does not contain tags so is not an issue (we don't use teaser anywhere by default)
3. In core AND Drupal CMS, the current markup does not work for the full view mode, which jumps from h1 to h3 for the article page

Does this clarify things? The fix would require us to handle the tags field differently for the teaser and full view modes.

@the_g_bomb this issue is for core only, we should handle any Drupal CMS exceptions separately.

Is this an improvement for core itself to make? I assume so, since it's using the same template. But if not, then we can close this and open CMS-only issues.

I don't follow. The home page would then go from h1 (once we re-add the title) to h3, isn't that the whole problem? Same for the listing pages, if we stick with h3 then it skips h2 on those pages.

Moved the variable to base.css because it will be needed by some of the XB components.

Could someone please split this out into actionable issues? Some of them should be fairly simple to address but they won't be all in one mega issue.

Autosave doesn't require an internet connection but it will fail if the site is unavailable. Are you using DDEV? If so, the site will break if you start with an internet connection and lose it, because of the routing magic. https://ddev.com/blog/ddev-name-resolution-wildcards/#what-happens-when-...

You can test this by turning off internet and trying to load the site, you'll get the same Chrome error.

Wow! Thank you for fixing and even doing a release so quickly.

Just tried to reproduce this on 11.x but I can't. On uninstall it updates the role config rather than deleting it.

Steps:

1. Install EB
2. Create a new EB
3. Grant permission to authenticated role to use new EB
4. Uninstall module
5. See warning that role config will be updated

@chris matthews the workaround would be to remove the permission(s) from the role before you uninstall the module.

Yeah, just wondered if anyone can verify that's the right approach as I am not sure.

This is annoyingly close and even 40% would work. I do think the grey is nicer than black so I added a custom style for this. Also updated the selectors so that it applies to all node types.

pameeela → made their first commit to this issue’s fork.

pameeela → created an issue.

Unfortunately automated checkers are looking for it and so we are shipping Drupal CMS with something that will be flagged as an error on all blog posts.

I'd certainly prefer to address this properly rather than just make the error go away by whatever means we can. So I still think changing it to an h2 is a better option and seems optimal as a change in core.

The fact that the Drupal CMS home page doesn't have an h1 seems like something to address separately, so I'll create an issue for that.

Found it :) https://yro.slashdot.org/story/07/09/25/1632246/wordpress-23-does-not-sp...

Sounds like this is covered then!

Thanks for looking into it @hchonov! I have not been able to reproduce it reliably, I just tried a fresh install twice (once via UI and once via drush) and I'm not seeing it now. Even when I did see it in the past, it only seemed to happen once on the first load.

If anyone else who saw it could provide more info or possibly test the patch that would be great!

Moving to Autosave Form queue. Does anyone have additional info about the error?

That sounds good! So no action needed here if that gets merged right? Assuming PB tags a new release before our next release?

Yeah, definitely don't think the IA of Drupal CMS should be different from core. So this would need to happen there. I'm not sure I agree though, for my part I certainly do not need to access menus anywhere near as often as content. Generally when I do update them I use the contextual links.

Great idea, I had the same one! This is a task for drupal.org though not Drupal CMS itself.

Works as expected with the updated config.

pameeela → created an issue.

Oh wait, I guess this is different, I've only seen it on the first load of the edit form -- don't think I have seen it on node/add.

I have seen this once or twice. It seems to occur when editing default content provided by a recipe, immediately after applying it. I was never able to reproduce it reliably though.

pameeela → created an issue.

pameeela → created an issue.

pameeela → created an issue.

Agreed, this would be great to have!

@tim.plunkett are you saying we should remove it from top tasks completely because it doesn't work in the trial? Hmmm.

Thanks, moving over to Gin for the fix!

@andrewbelcher agreed it is not ideal at the moment, but it's a fix needed in the Klaro module, not Drupal CMS itself. We discussed it a few days ago and I've just created an issue for it over there.

pameeela → created an issue.

@joelseguin thanks for finding and logging this :) Would you be able to create an MR for the fix? You just need to export the updated config and update the file in the content_type_base recipe.

I think it would be fine to just disable this link, unless someone feels strongly there is value in providing a generic RSS feed.

ckrina → credited pameeela → .

ckrina → credited pameeela → .

I don't think this is the right fix, it's still quite rigid in assuming the structure of this view, which can be modified to suit any site. I think perhaps we should remove these percentage based styles altogether, I think the table looks OK without them.

Not sure what's up here, I tested this locally and on the new trial experience and it worked as expected.

I think this all got done in other places, so closing to avoid any confusion.

I think this all got done in other places, so closing to avoid any confusion.

I think this all got done in other places, so closing to avoid any confusion.

I think this was probably in the WASM days, don't think it's needed with the hosted trial.

Closing since this hasn't been worked on in a while. Credited all those who contributed.

Pretty sure this has all been addressed, or at least has a plan, so we don't need this issue.

Agreed, thanks everyone for your input :)

Based on the discussion in #3333401-30: Pager h4 causes accessibility flag on many pages → it should be a heading. That comment also suggests it doesn't matter that much whether the headings increase by one.

But I guess h2 would work in all cases pretty much. I'm a bit confused though because the last comment says it's changed to a span but the MR has a div.

Looks "good enough" to me! Certainly better than now, which is no integration at all.

Just noting this component isn't intended for XB. So if we add info just to avoid confusion, we'll need to ensure it doesn't result in the component actually appearing there.

As I said in the other issue, I’m not sure that we will add content type, but I do think the styling here should be updated to be more flexible.

Thanks everyone for contributing to this already! Just wanted to say I’m not sure that we will want to make this change, I can see the use of it but it’s also nice that the dashboard is uncluttered.

We’re in a freeze for now and will weigh up the open issues when we resume.

This is a known issue in Autosave.

pameeela → created an issue.

This is a legitimate UX problem, but I'd be hesitant to introduce a solution only for CKEditor. It's also an issue with media fields and we'd need a separate solution for that.

I would also say this particular pattern of opening existing media in a modal might give the impression you are editing it within this context, rather than editing the media item itself, which may be referenced in other places.