Org settings layout fix + misc styling & consistency improvements

[emma-sg]

General changes

• Added postcss-lit, which allows us to use tailwind in lit elements with shadow DOMs
• Added // postcss-lit-disable-next-line comments to most css`...` tagged templates so as not to change existing CSS in components
• Added TailwindElement, which uses a single shared CSSStyleSheet across all instances to be able to access Tailwind without requiring a full copy of (compiled) Tailwind for every instance of a component that extends it
• Added a new <btrix-copy-field> element, replacing the existing copy elements

Org settings page

• Stopped content from overflowing at medium widths
• Made spacing consistent at both smaller and wider widths
• Used readonly/monospace styling for copyable org id field
• Updated tab shadows to be slightly blue, consistent with the tab background (also did this in other places tabs show up)

Before	After

Misc fixes

• Used consistent single-line readonly/monospace styling for copyable url field

Before	After

• Removed inconsistent angled bottom borders from crawl workflow list header

Before	After

• Changes all list page primary action buttons to use variant="primary"

[Shrinks99]

Consistency fixes are nice! I suppose we might want to add this to the crawl detail page as well?

Code for above:

<btrix-desc-list-item label=${msg("Crawl ID")}>
          ${this.crawl
            ? html`
                  <sl-input
                  class="mb-2 input-font-monostyle"
                  size="small"
                  readonly
                  filled
                  value=${this.crawl.id}
                  @sl-focus=${(e: FocusEvent) => {
                    (e.currentTarget as HTMLInputElement | undefined)?.select();
                  }}
                >
                  <btrix-copy-button
                    slot="suffix"
                    class="m-0"
                    hoist
                    value=${this.crawl.id}
                    content=${msg("Copy Public URL")}
                  ></btrix-copy-button>
                </sl-input>
              `
            : html`<sl-skeleton class="h-6"></sl-skeleton>`}
        </btrix-desc-list-item>

---

...Or perhaps not. The goal of matching our styling of our btrix-code element background is a good one, and I like the way it looks, but is using input elements the right choice here? Screen readers will read these as inputs which they aren't really. Also the fields have selection states whereas probably only the copy button and link visit button in the share menu should?

Keyboard tab navigation seems broken on the collection share menu... Not that it works properly in main either, but looks like a different reason?

[SuaYoo]

I'm getting the following runtime error from multiple files after rebasing main:

Module build failed (from ./node_modules/postcss-loader/dist/cjs.js):
TypeError: Property consequent of ConditionalExpression expected node to be of a type ["Expression"] but instead got undefined

How do the CSS changes impact final dist file sizes?

[emma-sg]

How do the CSS changes impact final dist file sizes?

Great question! I'm not actually sure, I suspect somewhat significantly. With shadow-root-enabled components we simply don't get the benefits of atomic CSS used across a codebase — as far as I understand, each shadow DOM needs its own complete css declaration as things are now. With Tailwind we could feasible share a single style sheet across the document root as well as all the shadow DOMs if we override or extend Lit's CSS handling and adopt a shared style sheet in TailwindElement, but that'd likely involve figuring out how to run PostCSS on this shared style sheet — maybe that's something we could do with raw-loader?

[emma-sg]

is using input elements the right choice here?

@Shrinks99 is it not? I'm not really sure, to be honest — in this case, there's a number of aspects that are akin to a form element: there's a label, the text is a self-contained piece of content, and there's already a readonly attribute on input elements that communicates that the element is a widget (and not just some more text) but cannot be changed.

[SuaYoo]

is using input elements the right choice here?

@Shrinks99 is it not? I'm not really sure, to be honest — in this case, there's a number of aspects that are akin to a form element: there's a label, the text is a self-contained piece of content

I think it makes sense in the context of a form, to indicate that the input value can be selected/focused on/copied. But could go either way since the value isn't sent to the server, and not sure it makes as much sense in the description list, since the <dt> is already providing a label, and the content of <dd> should be just the value--unsure how screen readers will approach the value of an input in <dd>.

[Shrinks99]

Just tried it in voice over, the big difference is that it reads out the content of the input with tab navigation because it's selected differently from the other boxes which don't do that (because they aren't user-editible elements / controls and therefore aren't included in the tabbing order) and the next item is the copy action, not the content. The prefix icon is currently skipped and tab navigation gets stuck and doesn't work for the suffix icon. After bit of looking into form labels and poking through the Firefox web inspector, I think the answer here is that you could be right if the elements above are properly attached to the input with aria-labelledby and aria-describedby... Which I don't think can really be done with Shoelace form elements? But I'd like to be wrong...

That being said, the current keyboard navigation and screen reader experience is far better. The link is read aloud with the first icon button and is correctly identified as a link, the second piece of text also stating the link should probably be given an aria-hidden attribute, and the copy button follows with a good description. I think everything in that section should also have an aria-group label too?? Would be in favour of changing back to the markup from main with new styling.

We really ought to do a pass on each page to make them properly navigable without double descriptions and such... I don't have time for this cleanup sprint, but I might for the next one!

[SuaYoo]

I'm getting the following runtime error from multiple files after rebasing main

Is this ready for re-review?

[emma-sg]

Is this ready for re-review?

@SuaYoo Should be, yes!

@Shrinks99 just to clarify — is the existing implementation (not using inputs) better for screen readers? If so I can quickly reimplement a component for this, rather than using inputs

[Shrinks99]

is the existing implementation (not using inputs) better for screen readers?

Yeah, using the input was def not as good.

Also if you're going to componentize this, depending on what actions are available, (if there's a button in the prefix slot with a link to visit the page) the text should be given aria-hidden because or else it will read it out twice... Assuming it's the same text and assuming it's always going to be a link. Perhaps we just add that manually depending on the context when implementing the component instead of trying to do anything fancy, but figured I'd note it. For the org ID example we wouldn't want to hide it because that's the actual content!

[emma-sg]

@Shrinks99 I'd love your thoughts on this impl!

[Shrinks99]

@Shrinks99 I'd love your thoughts on this impl!

Pretty friggin good! Yeet the old one from the share dialog and we're set!

[emma-sg]

Oh, yup, good catch!

[SuaYoo]

Why do we need to disable postcss-lit? I'm not seeing a visible difference disabling/enabling.

[emma-sg]

I don't know if we do — I'd put this disable everywhere I wasn't directly working on because I didn't want it to break anything unintentionally

[SuaYoo]

Looking good! One issue I'm seeing is with .sl-toast-stack positioning to the top right instead of bottom:

[emma-sg]

One issue I'm seeing is with .sl-toast-stack positioning to the top right instead of bottom

Should be fixed with 137bf4d in #1452!

[SuaYoo]

🔥

[emma-sg]

Re: build failure on 8bcca4d — I tested a build locally with ./scripts/build-frontend.sh and it worked after da59f77.