Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Emotion] Figure out what exports we do and don't want to make public #7025

Open
Tracked by #3912
cee-chen opened this issue Aug 1, 2023 · 0 comments
Open
Tracked by #3912
Labels
emotion task A task associated with a larger Meta issue

Comments

@cee-chen
Copy link
Member

cee-chen commented Aug 1, 2023

Currently, every single *.styles.ts component file is being exported in EUI as well as typed in eui.d.ts. As a team, we should tackle the following behaviors:

  1. Determine whether or not we as a team want to make component styles publicly

    • After a brief discussion in a team sync, we decided "not". Our current styles are incredibly specific to Emotion (due to the usage of css`` syntax - if we had used object styles instead, this would potentially be different/more flexible). It currently doesn't make much sense to make them reusable.
    • NOTE: If you have strong thoughts on/against the above, please feel free to leave a comment in this issue - we're open to discussing it!
    • Make it clear in our documentation that we only offer styled components going forward, not styles themselves (or rather, the style utilities we offer should be very broad and intentional and generally not component-specific)
    • However, before we can remove all .styles.ts files from being exported, we first need to account for consumers who are using Sass/CSS-in-JS variables & mixins that are component specific:
  2. Some components have style-related variables that may need to be reused by consumers, or even by other components within EUI. Examples of these are euiFormVariables and euiHeaderVariables.
    Instead of moving these style variables outside our .styles.ts files and continuing to export them publicly, @tkajtoch has suggested switching them to CSS variables instead. This will involve:

    1. Declaring these component style vars once at the top level in our global styles (ensuring consumers have access to the default vars everywhere in the app)
    2. Each component usage should check if modify exists in the nearest EuiThemeProvider and if it does, render new scoped definitions for each CSS var
    3. @tkajtoch also suggested adding a hook that exports the style vars later, although I'm not totally sure what his vision was in mind for that
  3. In our compile-eui and dtsgenerator scripts, ignore/skip all *.styles.ts files

  4. Audit & standardize all exported global_stylings/ mixins/functions
    EUI currently has several Emotion “hooks” we’re exporting, several decisions (or lack therefore) are confusing around them:

    1. Why some of them are hooks and some are not (we should likely eliminate all hooks and simply require the euiThemeContext to be passed in, this is also frankly easier for Emotion theme usage)
    2. Why some mixins/functions are in global_stylings and some are in theme/amsterdam
    3. Which should even be publicly available, and which we should make internal/EUI-only
@cee-chen cee-chen added emotion task A task associated with a larger Meta issue labels Aug 1, 2023
@github-actions github-actions bot added the Stale label Oct 4, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 11, 2024
@cee-chen cee-chen reopened this Oct 14, 2024
@cee-chen cee-chen removed the Stale label Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emotion task A task associated with a larger Meta issue
Projects
None yet
Development

No branches or pull requests

1 participant