Not sure what we want to go with, but we have some inconsistencies in our query keys right now. We want all query keys that involve organizations to have the exact same prefix so that they can grouped and invalidated together

Right now we're usingorganization (singular) andorganizations (plural). My gut feeling would be to use the plural version for everything, like you would for an HTTP route, but I don't have enough context

@aslilac Any preference for changing all query keys to be consistent?

I try to match the routes on the backend (which all do include the s)

@aslilac ok I will use plural here, but there are still several keys that do not match the routes,organizationMembersKey,getProvisionerDaemonGroupsKey,

Just curious: since we now have thebranding role, have there been any conversations about extending that down the line to work with customer-specific branding?

Nothing yet, so far this has purely been focused on the enterprise and premium badges and banners.

nit:theme.branding is not a role, it lives outside oftheme.roles, it's just an object/namespace

Have we decided on whether we prefer interfaces over type aliases? Asking because I generally avoid interfaces so that I can't do accidental interface merging, but I've been seeing a lot of recent code using interfaces

My goal was just to be consistent and I saw more interfaces used for all the organization related pages. I don't personally have a strong preference other than being consistent.

I likeinterface because I thinkinterface WibbleProps extends WobbleProps {} is a lot clearer thantype WibbleProps = WobbleProps & {} in the places where we need it. and I think we just generally use it in more places rn.

I know it's mock data, but just in case it has any bearing on regex code, this regex can be simplified as@\w+. The word class includes digits and underscores by default

the regex really can be anything, I did want to see a regex with a bit longer length.

Not sure how big of a deal this is, but the current implementation means that we can't add additional functionality without completely overriding the defaultsaveAs function. Every consumer would have to importsaveAs themselves, and then wire up the functionality for component implementation

How often would we have a use case where we'd want to get rid of thesaveAs functionality vs wanting to always havesaveAs, and then potentially add more functionality on top?

This was meant to just support the Storybook test case. Is there a better way to handle this?

This is dangerous – we need to be super, super careful with anyJSON.stringify calls inside components:

1. Stringifying on every render can be slow
2. JSON.stringify can throw, so if it fails, we might blow up the entire app

Have to put the comment here because the code was already in place previously, and it's not selectable for review:

I think we need to add some CSS optical adjustments to the key-value pairs for theIdpField component. Since we're using two different typefaces, their baselines are slightly different. And when you place them right next to each other on the same line, there's enough variation that the text looks sloppy

You might have to open this in a new tab, but the baselines are ever so slightly off, and the cap-heights are, too:

Though a slightly easier and probably much more manageable alternative would be to set all the text in monospace

The monospace font was supposed to be a temporary measure until this page becomes a form and these would become fields. I'll see if I can improve it for now.

as a temporary fix, I can add some padding to make the baselines match. I really wanted to use the monospace font on only the field value to signify that it is the value of the configuration.

Is this the right name for the prop if it's only being used to show disabled statuses?

I thought it may be used for more in the future but Ill rename it for now.

can you add this above thegetGroup... function too pls?

I try to match the routes on the backend (which all do include the s)

nit:theme.branding is not a role, it lives outside oftheme.roles, it's just an object/namespace

I likeinterface because I thinkinterface WibbleProps extends WobbleProps {} is a lot clearer thantype WibbleProps = WobbleProps & {} in the places where we need it. and I think we just generally use it in more places rn.

oh this is so gross (but I don't know what to suggest to make it better)

could you add a comment above it to elaborate why this incantation is justified/necessary? Ithink I understand what it's doing, but clarity is good, and more junior devs might not get it!