Skip to content

Commit

Permalink
Merge branch 'master' into rounding-3000
Browse files Browse the repository at this point in the history
  • Loading branch information
Twixes authored Nov 23, 2023
2 parents 2729a80 + b7501b1 commit b88a8a9
Show file tree
Hide file tree
Showing 101 changed files with 324 additions and 262 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ jobs:
# NOTE: we are at risk of missing a dependency here. We could make
# the dependencies more clear if we separated the backend/frontend
# code completely
# really we should ignore ee/frontend/** but dorny doesn't support that
- 'ee/**/*'
- '!ee/frontend/**'
- 'posthog/**/*'
- 'bin/*.py'
- requirements.txt
Expand Down
8 changes: 7 additions & 1 deletion .github/workflows/ci-frontend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ jobs:
# NOTE: we are at risk of missing a dependency here.
- 'bin/**'
- 'frontend/**'
- 'ee/frontend/**'
# Make sure we run if someone is explicitly change the workflow
- .github/workflows/ci-frontend.yml
# various JS config files
Expand Down Expand Up @@ -117,18 +118,23 @@ jobs:
jest:
runs-on: ubuntu-latest
needs: changes
name: Jest test (${{ matrix.chunk }})
name: Jest test (${{ matrix.segment }} - ${{ matrix.chunk }})

strategy:
# If one test fails, still run the others
fail-fast: false
matrix:
segment: ['FOSS', 'EE']
chunk: [1, 2, 3]

steps:
# we need at least one thing to run to make sure we include everything for required jobs
- uses: actions/checkout@v3

- name: Remove ee
if: needs.changes.outputs.frontend == 'true' && matrix.segment == 'FOSS'
run: rm -rf ee

- name: Install pnpm
if: needs.changes.outputs.frontend == 'true'
uses: pnpm/action-setup@v2
Expand Down
64 changes: 48 additions & 16 deletions .storybook/test-runner.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { toMatchImageSnapshot } from 'jest-image-snapshot'
import { getStoryContext, TestRunnerConfig, TestContext } from '@storybook/test-runner'
import { getStoryContext, TestRunnerConfig, TestContext, waitForPageReady } from '@storybook/test-runner'
import type { Locator, Page, LocatorScreenshotOptions } from '@playwright/test'
import type { Mocks } from '~/mocks/utils'
import { StoryContext } from '@storybook/types'
import { StoryContext } from '@storybook/csf'

// 'firefox' is technically supported too, but as of June 2023 it has memory usage issues that make is unusable
type SupportedBrowserName = 'chromium' | 'webkit'
type SnapshotTheme = 'legacy' | 'light' | 'dark'

// Extend Storybook interface `Parameters` with Chromatic parameters
declare module '@storybook/types' {
Expand Down Expand Up @@ -35,12 +36,18 @@ declare module '@storybook/types' {
snapshotBrowsers?: SupportedBrowserName[]
/** If taking a component snapshot, you can narrow it down by specifying the selector. */
snapshotTargetSelector?: string
/** Include snapshots of buttons in 3000. */
include3000?: boolean
}
msw?: {
mocks?: Mocks
}
[name: string]: any
}

interface Globals {
theme: SnapshotTheme
}
}

const RETRY_TIMES = 3
Expand Down Expand Up @@ -68,7 +75,7 @@ module.exports = {
},
async postVisit(page, context) {
const browserContext = page.context()
const storyContext = (await getStoryContext(page, context)) as StoryContext
const storyContext = await getStoryContext(page, context)
const { snapshotBrowsers = ['chromium'] } = storyContext.parameters?.testOptions ?? {}

browserContext.setDefaultTimeout(PLAYWRIGHT_TIMEOUT_MS)
Expand All @@ -92,12 +99,14 @@ async function expectStoryToMatchSnapshot(
waitForLoadersToDisappear = true,
waitForSelector,
excludeNavigationFromSnapshot = false,
include3000 = false,
} = storyContext.parameters?.testOptions ?? {}

let check: (
page: Page,
context: TestContext,
browser: SupportedBrowserName,
theme: SnapshotTheme,
targetSelector?: string
) => Promise<void>
if (storyContext.parameters?.layout === 'fullscreen') {
Expand All @@ -110,8 +119,7 @@ async function expectStoryToMatchSnapshot(
check = expectStoryToMatchComponentSnapshot
}

// Wait for story to load
await page.waitForSelector('.sb-show-preparing-story', { state: 'detached' })
await waitForPageReady(page)
await page.evaluate(() => {
// Stop all animations for consistent snapshots
document.body.classList.add('storybook-test-runner')
Expand All @@ -123,44 +131,62 @@ async function expectStoryToMatchSnapshot(
await page.waitForSelector(waitForSelector)
}

await page.waitForTimeout(400) // Wait for animations to finish
await page.waitForTimeout(400) // Wait for effects to finish

// Wait for all images to load
await page.waitForFunction(() =>
Array.from(document.querySelectorAll('img')).every((i: HTMLImageElement) => i.complete)
)

await check(page, context, browser, storyContext.parameters?.testOptions?.snapshotTargetSelector)
await check(page, context, browser, 'legacy', storyContext.parameters?.testOptions?.snapshotTargetSelector)

if (include3000) {
await page.evaluate(() => {
document.body.classList.add('posthog-3000')
document.body.setAttribute('theme', 'light')
})

await check(page, context, browser, 'light', storyContext.parameters?.testOptions?.snapshotTargetSelector)

await page.evaluate(() => {
document.body.setAttribute('theme', 'dark')
})

await check(page, context, browser, 'dark', storyContext.parameters?.testOptions?.snapshotTargetSelector)
}
}

async function expectStoryToMatchFullPageSnapshot(
page: Page,
context: TestContext,
browser: SupportedBrowserName
browser: SupportedBrowserName,
theme: SnapshotTheme
): Promise<void> {
await expectLocatorToMatchStorySnapshot(page, context, browser)
await expectLocatorToMatchStorySnapshot(page, context, browser, theme)
}

async function expectStoryToMatchSceneSnapshot(
page: Page,
context: TestContext,
browser: SupportedBrowserName
browser: SupportedBrowserName,
theme: SnapshotTheme
): Promise<void> {
await page.evaluate(() => {
// The screenshot gets clipped by the overflow hidden of the sidebar
document.querySelector('.SideBar')?.setAttribute('style', 'overflow: visible;')
})

await expectLocatorToMatchStorySnapshot(page.locator('.main-app-content'), context, browser)
await expectLocatorToMatchStorySnapshot(page.locator('.main-app-content'), context, browser, theme)
}

async function expectStoryToMatchComponentSnapshot(
page: Page,
context: TestContext,
browser: SupportedBrowserName,
theme: SnapshotTheme,
targetSelector: string = '#storybook-root'
): Promise<void> {
await page.evaluate(() => {
await page.evaluate((theme) => {
const rootEl = document.getElementById('storybook-root')
if (!rootEl) {
throw new Error('Could not find root element')
Expand All @@ -184,21 +210,27 @@ async function expectStoryToMatchComponentSnapshot(
rootEl.style.width = `${-popoverBoundingClientRect.left + currentRootBoundingClientRect.right}px`
}
})
// Make the body transparent to take the screenshot without background
document.body.style.background = 'transparent'
})
// For legacy style, make the body transparent to take the screenshot without background
document.body.style.background = theme === 'legacy' ? 'transparent' : 'var(--bg-3000)'
}, theme)

await expectLocatorToMatchStorySnapshot(page.locator(targetSelector), context, browser, { omitBackground: true })
await expectLocatorToMatchStorySnapshot(page.locator(targetSelector), context, browser, theme, {
omitBackground: true,
})
}

async function expectLocatorToMatchStorySnapshot(
locator: Locator | Page,
context: TestContext,
browser: SupportedBrowserName,
theme: SnapshotTheme,
options?: LocatorScreenshotOptions
): Promise<void> {
const image = await locator.screenshot({ ...options })
let customSnapshotIdentifier = context.id
if (theme !== 'legacy') {
customSnapshotIdentifier += `--${theme}`
}
if (browser !== 'chromium') {
customSnapshotIdentifier += `--${browser}`
}
Expand Down
13 changes: 13 additions & 0 deletions ee/frontend/exports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { PostHogEE } from '@posthog/ee/types'

const myTestCode = (): void => {
// eslint-disable-next-line no-console
console.log('it works!')
}

const postHogEE: PostHogEE = {
enabled: true,
myTestCode,
}

export default postHogEE
7 changes: 7 additions & 0 deletions frontend/@posthog/ee/exports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { PostHogEE } from './types'

const posthogEE: PostHogEE = {
enabled: false,
}

export default posthogEE
5 changes: 5 additions & 0 deletions frontend/@posthog/ee/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// NOTE: All exported items from the EE module _must_ be optionally defined to ensure we work well with FOSS
export type PostHogEE = {
enabled: boolean
myTestCode?: () => void
}
Binary file modified frontend/__snapshots__/components-compact-list--compact-list.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-select--long-options.png
Binary file modified frontend/__snapshots__/lemon-ui-utilities--overview.png
Binary file modified frontend/__snapshots__/scenes-other-toolbar--heatmap-dark.png
18 changes: 9 additions & 9 deletions frontend/src/lib/components/CompactList/CompactList.scss
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
.compact-list {
.CompactList {
border-radius: var(--radius);
border: 1px solid var(--border);
height: calc(19.25rem);
background: var(--bg-light);
box-sizing: content-box;
display: flex;
flex-direction: column;
flex: 1;
overflow: hidden;

.compact-list-header {
padding: 0.5rem 1rem 0;
.CompactList__header {
padding: 0.5rem;
display: flex;
align-items: center;
justify-content: space-between;

h3 {
margin-bottom: 0;
font-weight: 600;
font-size: 1rem;
font-size: 0.9rem;
line-height: 1.4;
}
}

.scrollable-list {
flex: 1;
overflow: auto;
padding: 0 0.5rem;
.CompactList__content {
overflow-y: auto;
height: 16rem;
padding: 0.5rem;
}

.LemonButton {
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/lib/components/CompactList/CompactList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ export function CompactList({
renderRow,
}: CompactListProps): JSX.Element {
return (
<div className="compact-list border">
<div className="compact-list-header">
<h3>{title}</h3>
<div className="CompactList">
<div className="CompactList__header">
<h3 className="px-2 truncate" title={title}>
{title}
</h3>
{viewAllURL && <LemonButton to={viewAllURL}>View all</LemonButton>}
</div>
<div className="mx-2">
<LemonDivider />
</div>
<div className="scrollable-list">
<LemonDivider className="my-0 mx-2" />
<div className="CompactList__content">
{loading ? (
<div className="p-2 space-y-6">
{Array.from({ length: 6 }, (_, index) => (
Expand Down
17 changes: 17 additions & 0 deletions frontend/src/lib/ee.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import fs from 'fs'

const eeFolderExists = fs.existsSync('ee/frontend/exports.ts')
export const ifEeIt = eeFolderExists ? it : it.skip
export const ifFossIt = !eeFolderExists ? it : it.skip

import posthogEE from '@posthog/ee/exports'

describe('ee importing', () => {
ifEeIt('should import actual ee code', () => {
expect(posthogEE.enabled).toBe(true)
})

ifFossIt('should import actual ee code', () => {
expect(posthogEE.enabled).toBe(false)
})
})
3 changes: 3 additions & 0 deletions frontend/src/lib/lemon-ui/LemonButton/LemonButton.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ const meta: Meta<typeof LemonButton> = {
type: 'function',
},
},
parameters: {
testOptions: { include3000: true },
},
}
export default meta
const BasicTemplate: StoryFn<typeof LemonButton> = (props: LemonButtonProps) => {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/scenes/funnels/FunnelBarGraph/Bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface BarProps {
wrapperWidth: number
}
type LabelPosition = 'inside' | 'outside'

export function Bar({
percentage: conversionPercentage,
name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export function FunnelBarGraph({
) : null}
</header>
<div className="funnel-inner-viz">
<div className={clsx('funnel-bar-wrapper', { breakdown: isBreakdown })}>
<div className={clsx('funnel-bar-wrapper', { breakdown: isBreakdown })} aria-busy={!width}>
{!width ? null : isBreakdown ? (
<>
{step?.nested_breakdown?.map((breakdown, index) => {
Expand Down
19 changes: 14 additions & 5 deletions frontend/src/scenes/notebooks/Nodes/NotebookNodeQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ const DEFAULT_QUERY: QuerySchema = {
},
}

const Component = ({ attributes }: NotebookNodeProps<NotebookNodeQueryAttributes>): JSX.Element | null => {
const Component = ({
attributes,
updateAttributes,
}: NotebookNodeProps<NotebookNodeQueryAttributes>): JSX.Element | null => {
const { query, nodeId } = attributes
const nodeLogic = useMountedLogic(notebookNodeLogic)
const { expanded } = useValues(nodeLogic)
Expand Down Expand Up @@ -86,10 +89,17 @@ const Component = ({ attributes }: NotebookNodeProps<NotebookNodeQueryAttributes
return (
<div className={clsx('flex flex-1 flex-col h-full')}>
<Query
query={modifiedQuery}
// use separate keys for the settings and visualization to avoid conflicts with insightProps
uniqueKey={nodeId + '-component'}
readOnly={true}
query={modifiedQuery}
setQuery={(t) => {
updateAttributes({
query: {
...attributes.query,
source: (t as DataTableNode | InsightVizNode).source,
} as QuerySchema,
})
}}
/>
</div>
)
Expand Down Expand Up @@ -180,10 +190,9 @@ export const Settings = ({
) : (
<div className="p-3">
<Query
query={modifiedQuery}
// use separate keys for the settings and visualization to avoid conflicts with insightProps
uniqueKey={attributes.nodeId + '-settings'}
readOnly={false}
query={modifiedQuery}
setQuery={(t) => {
updateAttributes({
query: {
Expand Down
Loading

0 comments on commit b88a8a9

Please sign in to comment.