🇺🇸United States @andy-blum

smustgrave → credited andy-blum → .

smustgrave → credited andy-blum → .

andy-blum → made their first commit to this issue’s fork.

Kristen Pol → credited andy-blum → .

nod_ → credited andy-blum → .

In the beginning, all of these actions are achieved through a low-code experience using a code editor within the browser.

If part of the rationale for low-code, in-browser editing is to help ease new Drupal users into the developer role, I wonder if it makes sense & would be possible to ease them into industry-standard tools as well. Could the in-browser code editor be vscodum-based with some well-curated extensions? This could also help pipe new drupal users into contribution via DrupalPod.

Have you considered allowing users to just set a path to an svg similar to theme favicon/logo fields? The current icons are set in CSS via mask-image: var(--icon); and --icon is set to url("data:image/svg+xml, <some svg code>").

If custom menu items are added and have a field that takes a path to an icon, we could use file_get_contents() to get the SVG code and then urlencode() to convert it to the format CSS expects. Then setting style="--icon: ..." on the custom menu item allows no custom CSS to be written.

I'm honestly not sure the "X" mark is intentional, but if it's not, I think a better path would be to move to using input[type="text"] over input[type="search"]. They're almost functionally identical with only a couple of key differences. This would certainly be an easier fix than the soup of browser prefixes added in patch #3:

/* removing cross for the mobile */

.block-search-narrow [type="search"]::-webkit-search-cancel-button,
.block-search-narrow [type="search"]::-webkit-search-clear-button {
  display: none;
}

.block-search-narrow [type="search"]::-moz-search-clear-button {
  display: none;
}

.block-search-narrow [type="search"]::-ms-clear {
  display: none;
}

If we were to change this from a search input to a text input, we'd just need to make sure that we mimic the search type's accessibility values, namely, changing the role of the input to "searchbox" to match the search input's implicit aria-role value

andy-blum → changed the visibility of the branch 10.1.x to hidden.

@platinum1 can you re-test and confirm if this issue still exists? If it does, can you provide more info on the device type & browser being used?

Now that 📌 Making a theme compatible with core's theme generator is too difficult Needs work is in, this is unblocked!

@mglaman - It might be worth fleshing out the bullet point about the info.yml file. Specifically, that you can override existing values and *remove* values with `null`.

nod_ → credited andy-blum → .

Adding some findings to the IS.

Downgrading to normal priority as this bug doesn’t fit the criteria of major

https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →

Gábor Hojtsy → credited andy-blum → .

One thing that we may want to consider & will start to get tricky is browser testing on iOS. As of iOS 17.4, iOS will begin to allow browser engines other than webkit. The requirements for this are pretty strict right now, but there's the potential for browser applications availalbe in the EU that aren't available anywhere else.

You should definitely be able to run the command inside ddev.

There are a couple ways to accomplish this:

1. Run ddev ssh. This will allow you to run commands inside the container.
2. Run ddev exec php [command]. This will pass the command into the container and then running it without changing your entire CLI environment

The tricky part of the second version is you have to know what the current working directory is. By default it's /var/www/html/, and if your drupal installation is inside a "web" directory, you have to add that part to your command.

Try running ddev exec php web/core/scripts/drupal generate-theme mytheme and see if that works.

Core/scripts/drupal is an extensionless file, not a folder. It is the 7th item in your screenshot.

Adding a comment here just to bump this issue back to the top of my dashboard.

inverted-colors is widely unsupported. I'd suggest removing it from your list.

Test failures appear to be aligned with current failures against 1.0.x

Moved Phil's patch into the MR and fixed PHPCS errors

andy-blum → made their first commit to this issue’s fork.

The MR changes code only in Claro, so updating this to the Claro issue queue.

nicxvan → credited andy-blum → .

Typically code changes should be submitted as patches or MRs - that way the testing infrastructure can review the code as well. Your CSS, however, is not a bug fix. Your CSS is a change based on your opinion of a design that was finalized several years ago, and that is in active use in sites. Changing this design in Olivero now would cause regressions in existing sites. As such, I have continually marked this as "works as designed".

Again, if this change is something you feel that strongly about, I recommend using the starterkit patch provided above or some other method for creating your own custom solution.

The empty column is there because the website layout has been deliberately limited to a max-width of 98.125rem. Without this limitation, the website would spread across the entire screen hindering readability of the content and causing large swaths of empty space between the content, the logo, and the navigation.

If this is not acceptable to you, you are free to make a new theme using Olivero as a starterkit 📌 [PP-1] Allow starterkit theme generator tool to clone Olivero Postponed . This design of Olivero has been finalized for quite some time now and it will not be changing. Please stop re-opening this issue.

Page content is limited in width for readability.
https://css-tricks.com/six-tips-for-better-web-typography/

Page layout is limited in width to keep the rest of the page elements in the same general area with the page content.

If you think this layout would look better center aligned, I'd direct you to a different issue, 📌 Olivero: Center align content (instead of left align) Needs work .

we still want to convert values to custom properties

Is this necessary since the classes aren't obfuscated, the styles are BEMified, and everything is brought in via the Library API? I think once the plan outlined in #3394904 is done and the styles in PB are reduced to just basic layout styles (similar to the views module when viewed in Stark), admin themes will be responsible for theming the PB UI.

allow prefix/suffix inserting with Drupal.theme()

This probably needs someone investigating if this can be done safely. I imagine Drupal.theme() will break a lot of the svelte event bindings.

andy-blum → created an issue.

chrisfromredfin → credited andy-blum → .

Marked as ready to kick off an initial round of tests to make sure I'm on the right track.

andy-blum → made their first commit to this issue’s fork.

This issue feels nullified by the resolution outlined in the issue description on 📌 Move styles out of svelte bundle to traditional Drupal library Fixed

Is this issue still relevant after 📌 Move styles out of svelte bundle to traditional Drupal library Fixed

The changes to <MediaQuery> can be seen in isolation here

The goal here is to avoid needing to add in duplicate MediaQuery components to get effects at the same breakpoint. The MQ component now adds that value into a Map where the key is the exact media query string and the value is a boolean.

With this logic, we can now update nested components to simply reference and respond to the values in the store like in this commit

The exact reason this was needed in the first place is that the project grid/list toggles back and forth between the two for both user intiated reasons, as well as automatically when the browser is resized. Prior to this change the following could happen:

• User loads page on desktop
• User toggles to list view
• User resizes browser down to "mobile" breakpoint
• User resizes browser up to "desktop" breakpoint and the layout is automatically switched back to grid

Now, the value of toggleView is always directly linked to the user's specified preference and we're able to override that when the viewport width necessitates it.

In a highly-simplified version of the code:

ProjectBrowser.svelte handles the media query as well as the user preferences from the list/grid buttons.

<MediaQuery query="(min-width: 1200px)" let:matches>
  <ProjectGrid {toggleView} {loading} {rows} {pageIndex} {$pageSize} let:rows>
    <button on:click={}>List</button>
    <button on:click={}>Grid</button>

    {#each rows as row, index (row)}
      <Project {toggleView} project={row} />
    {/each}
    
  </ProjectGrid>
</MediaQuery>

ProjectGrid.svelte adds the preferred class on desktop breakpoints and the list class on smaller widths

<ul class="projects-{isDesktop ? toggleView.toLowerCase() : 'list'}">
  <slot rows={visibleRows} />
</ul>

Project.svelte does the same thing as ProjectGrid.svelte

<li class="project project--{isDesktop ? toggleView.toLowerCase() : 'list'}">
  
</li>

Closing this as duplicate because we're adding this as part of 📌 Making a theme compatible with core's theme generator is too difficult Needs work

Closing this as duplicate because we're adding this as part of 📌 Making a theme compatible with core's theme generator is too difficult Needs work

Test failures are now related to the generate theme command. Abridged failures below:

---- Drupal\Tests\Core\Command\GenerateThemeTest ----
    
Testing Drupal\Tests\Core\Command\GenerateThemeTest
FFFFFFF..                                                           9 / 9
(100%)

Time: 00:56.152, Memory: 12.00 MB

There were 7 failures:

1) Drupal\Tests\Core\Command\GenerateThemeTest::test
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'starterkit_theme:9.4.0'
+'test_custom_theme:1.0.0'


2)
Drupal\Tests\Core\Command\GenerateThemeTest::testGeneratingFromAnotherTheme
In InfoParserDynamic.php line 67:

  The 'core_version_requirement' key must be present in themes/test_custom_theme/test_custom_theme.info.yml


generate-theme [--name [NAME]] [--description [DESCRIPTION]] [--path
[PATH]] [--starterkit [STARTERKIT]] [--] 


Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Theme generated successfully to themes/generated_from_another_theme'
+''


3) Drupal\Tests\Core\Command\GenerateThemeTest::testDevSnapshot
Failed asserting that 'test_custom_theme:1.0.0' matches PCRE pattern
"/^starterkit_theme\:9.4.0-dev#[0-9a-f]+$/".


4) Drupal\Tests\Core\Command\GenerateThemeTest::testContribStarterkit
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'starterkit_theme:1.20'
+'test_custom_theme:1.0.0'


5) Drupal\Tests\Core\Command\GenerateThemeTest::testContribStarterkitDevSnapshot
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'The source theme starterkit_theme has a development version number
(7.x-dev). Because it is not a git checkout, a specific commit could not be
identified. This makes tracking changes in the source theme difficult. Are
you sure you want to continue? (yes/no) [yes]:\n
- > Theme generated successfully to themes/test_custom_theme'
+'Theme generated successfully to themes/test_custom_theme'

6) Drupal\Tests\Core\Command\GenerateThemeTest::testContribStarterkitDevSnapshotWithGitNotInstalled
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'[ERROR] The source theme starterkit_theme has a development version
number     \n
-         (7.x-dev). Determining a specific commit is not possible because
git is\n
-         not installed. Either install git or use a tagged release to
generate a\n
-         theme.'
+'Theme generated successfully to themes/test_custom_theme'

7) Drupal\Tests\Core\Command\GenerateThemeTest::testCustomStarterkit
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'The source theme starterkit_theme does not have a version specified. This
makes tracking changes in the source theme difficult. Are you sure you want
to continue? (yes/no) [yes]:\n
- > Theme generated successfully to themes/test_custom_theme'
+'Theme generated successfully to themes/test_custom_theme'

FAILURES!
Tests: 9, Assertions: 32, Failures: 7.

Symfony's finder & filesystem components are now full runtime dependencies [ CR → ]

Symfony's finder & filesystem components are now full runtime dependencies [ CR → ]

I ran these tests locally and was unable to replicate the errors associated with #5e6988c5. I wanted to re-run the tests, but I don't see that as an option anymore on the d.o test results page.

I've updated a CSS selector for the sake of making a new push instead.

I think your option 4 is the same as my option 2, though?

I love this idea, and think an addition to the libraries.yml would be the perfect place to declare es module usage. For example from the react module's current usage of the importmaps module:

react.libraries.yml

react:
  js:
    'js/dist/react.js': { minified: true }

react-dom:
  js:
    'js/dist/react-dom.js': { minified: true }
  dependencies:
    - react/react

react.importmaps.yml

react:
  path: js/dist/react.js
react-dom:
  path: js/dist/react-dom.js
react/jsx-runtime:
  path: js/dist/react-jsx-runtime.js

To kick off some discussion here, re-imagining this as a single .libraries.yml file could become one of several options, each with their own benefits/drawbacks.

Option 1: Adding importpath info to a JS asset

react:
  js:
    'js/dist/react.js': { minified: true, esmodule: 'react' }

react-dom:
  js:
    'js/dist/react-dom.js': { minified: true, esmodule: 'react-dom' }
  dependencies:
    - react/react

Pros:

• Minimal addition to the yaml structure
• Tight coupling to current libraries

Cons:

• JS files don't necessarily need to be loaded on a page for the import map to work, but this approach would add them into the DOM 100% of the time

Option 2: Adding importpath info to an individual library

react:
  js:
    'js/dist/react.js': { minified: true }
  imports:
    'react': 'js/dist/react.js' 

react-dom:
  js:
    'js/dist/react-dom.js': { minified: true }
  imports:
    'react-dom': 'js/dist/react-dom.js'
  dependencies:
    - react/react

Pros:

• Moderate coupling to current libraries
• Files that exist only as "sources" of modules won't get unnecessarily loaded to the page

Cons:

• Files that both need to both run *and* operate at a module's source have to be duplicated

Option 3: Adding importpath info as a separate non-library top level item

react:
  js:
    'js/dist/react.js': { minified: true }

react-dom:
  js:
    'js/dist/react-dom.js': { minified: true }
  dependencies:
    - react/react

IMPORT_PATH:
  'react': 'js/dist/react.js'
  'react-dom': 'js/dist/react-dom.js'

Pros:

• Offers the greatest flexibility

Cons:

• Loosest coupling to libraries
• Requires a "magic" library name

We are moving in the wrong direction.

andy-blum → created an issue.

@deborahblessy

As Eli said, thank you for contributing!

I took a quick look at your patch and wanted point out that while {3,6} does match the 3-character syntax for hex codes, the regex as you've written it will also match to invalid 4- and 5-character strings. Additionally, the HTMLColorInput that we create in color-picker.js will only accept a full 6-character string, so we have to also flesh out any 3-character strings before we try to set that element's value.

Since your patch passes the tests but would cause problems in real situations, we probably have a need for some additional tests. I'll open a followup issue for those.

andy-blum → made their first commit to this issue’s fork.

The change to the JS shouldn't have any side-effects. I agree with @GoZ's assessment that running the JS in strict mode is likely the cause of the issue. At Lullabot, we've made the decision to write all JS using strict mode, but likely haven't encountered this exact error since I don't believe we're using advagg.

in attempting to follow @GoZ's instructions, I couldn't reproduce the issue. That being said, using `this` outside of object-oriented code raises red flags and I think this change to a more explicit `window` is a good decision. I've attached some screenshots to hopefully demonstrate that the changes of `this` to `window` in the scope of an IIFE do not impact

Is this issue still relevant? Should it be replaced by 📌 Refactor Claro's dropbutton stylesheet Needs review ?

Opened ✨ Add Symfony's Filesystem and Finder components to core Active for the work following this dependency evaluation.

Opened ✨ Add Symfony's Filesystem and Finder components to core Active for the work following this dependency evaluation.

andy-blum → created an issue.

andy-blum → made their first commit to this issue’s fork.

There are two primary arguments I have against requiring core-dev as a solution to this issue:

1. core-dev is not bundled into the tarballs which can be downloaded from Drupal core's project page - in fact there's no messaging on that page at all advocating for composer installation as a preferred method.
2. The starterkit tool is grouped with recipes and distributions as a tool to reduce the barrier of entry for newer and less experienced developers. I don't believe requiring core-dev is in line with serving that audience, even if it's a relatively easy task to add/remove.

Assigning this to myself to work on at DrupalCon Lille.

andy-blum → created an issue.

While the example provided here isn't valid, it does highlight a problem that could be experienced with very long words on the mobile layout. For example, if we add a very long german word, we can experience the overflow issue identified here.

By adding the hyphens: auto; CSS rule, we can get that word to break in a language specific way.

Language support depends on browser implementation, which is poor according to MDN, but that information appears to be out-of-date

Per the designs, as the screen size shrinks, the sidebar is moved below the main content and is allowed to take up more space on the grid container. If you have design suggestions, please share them, but as-is, I don't believe this is an issue.

andy-blum → created an issue.

nicxvan → credited andy-blum → .

nicxvan → credited andy-blum → .

would it be possible to alphabetize the components as well?

nicxvan → credited andy-blum → .

nicxvan → credited andy-blum → .

andy-blum → made their first commit to this issue’s fork.

There is currently an option for "custom location" that does nothing right now. It's effectively identical to the "bundled version" option.

I'm envisioning something similar to the favicon & logo settings where admins can set their own paths.

Removed outdated info from "about" section.

Added settings to use bundled assets or CDN assets on any version.

andy-blum → created an issue.

I am qualified to review the CSS 😁

Everything here looks fine. The nits below are completely optional.

.banner-block__image img {
    position: absolute;
    top: 0;
    left: 0;
    width: 100%;
    height: 100%;
    object-fit: cover;
}

"top" and "left" could be replaced with a single "inset:0;". This reduces the CSS by a negligible amount and adheres to the best practices of logical properties

.banner-block__content {
    position: relative; /* Establish stacking context to appear above absolutely positioned background image. */
    max-width: 50%;
    margin: 0;
    padding: 1.777em;
    color: #fff;
    border: 1px solid #464646;
    background: rgba(0, 0, 0, 0.42);
}

This is a super pedantic nit, but these rules don't actually create a new stacking context, they just position the content properly (additional info)

ckrina → credited andy-blum → .

Is there a reason (either way) to introduce/not introduce them?

The reason for trying to avoid the new dependencies was to speed this along.

If the code is really complicated, does this help make the case for including the dependencies?

I would argue yes.

Ready for review.

I don't believe this is actually an issue, is it? uikit/global-style depends on uikit/uikit, which already declares core/jquery and core/drual as dependencies. Similarly, uikit/active-link depends on uikit/uikit, which depends on core/drupal, which depends on core/drupalSettings

Jquery is needed, but jquery_ui/_dialog are not. Removing these as part of 🐛 Drupal 10 support currently broken Fixed .

Closing in favor of 🐛 Drupal 10 support currently broken Fixed

The page content is in a wrapper with a set max width. The “drops” pattern is the background that takes up the remainder of the page instead of blank white.