-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(rename-sql-editor-tabs): Allows renaming of tabs #27608
base: master
Are you sure you want to change the base?
Changes from all commits
3a32336
d4ecd38
628180f
3773d5c
1b9da50
dbafca4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import React, { useEffect, useRef } from 'react' | ||
|
||
interface AutoTabProps { | ||
value: string | ||
onChange: React.ChangeEventHandler<HTMLInputElement> | ||
onKeyDown?: React.KeyboardEventHandler<HTMLInputElement> | ||
onBlur: React.FocusEventHandler<HTMLInputElement> | ||
handleRename: () => void | ||
autoFocus?: boolean | ||
} | ||
|
||
/** | ||
* Tab component that automatically resizes an input field to match the width of its content based upon | ||
* the width of a hidden span element. | ||
*/ | ||
const AutoTab = ({ value, onChange, onKeyDown, onBlur, autoFocus, handleRename }: AutoTabProps): JSX.Element => { | ||
const inputRef = useRef<HTMLInputElement>(null) | ||
const spanRef = useRef<HTMLSpanElement>(null) | ||
|
||
useEffect(() => { | ||
if (!inputRef.current || !spanRef.current) { | ||
return | ||
} | ||
const newWidth = spanRef.current.offsetWidth | ||
inputRef.current.style.width = newWidth + 'px' | ||
}, [value]) | ||
|
||
const handleChange = (e: React.ChangeEvent<HTMLInputElement>): void => { | ||
onChange(e) | ||
handleRename() | ||
} | ||
|
||
return ( | ||
<div className="relative inline-block"> | ||
<span ref={spanRef} className="pointer-events-none absolute invisible whitespace-pre" aria-hidden="true"> | ||
{value} | ||
</span> | ||
<input | ||
ref={inputRef} | ||
className="bg-transparent border-none focus:outline-none p-0" | ||
value={value} | ||
onChange={handleChange} | ||
onKeyDown={onKeyDown} | ||
onBlur={onBlur} | ||
autoFocus={autoFocus} | ||
/> | ||
</div> | ||
) | ||
} | ||
|
||
export default AutoTab |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,21 @@ | ||
import { IconPlus, IconX } from '@posthog/icons' | ||
import { LemonButton } from '@posthog/lemon-ui' | ||
import clsx from 'clsx' | ||
import { useState } from 'react' | ||
|
||
import { QueryTab } from './multitabEditorLogic' | ||
import AutoTab from './AutoTab' | ||
import { NEW_QUERY, QueryTab } from './multitabEditorLogic' | ||
|
||
interface QueryTabsProps { | ||
models: QueryTab[] | ||
onClick: (model: QueryTab) => void | ||
onClear: (model: QueryTab) => void | ||
onRename: (model: QueryTab, newName: string) => void | ||
onAdd: () => void | ||
activeModelUri: QueryTab | null | ||
} | ||
|
||
export function QueryTabs({ models, onClear, onClick, onAdd, activeModelUri }: QueryTabsProps): JSX.Element { | ||
export function QueryTabs({ models, onClear, onClick, onAdd, onRename, activeModelUri }: QueryTabsProps): JSX.Element { | ||
return ( | ||
<div className="flex flex-row w-full overflow-scroll hide-scrollbar h-10 pt-1"> | ||
{models.map((model: QueryTab) => ( | ||
|
@@ -22,6 +25,7 @@ export function QueryTabs({ models, onClear, onClick, onAdd, activeModelUri }: Q | |
onClear={models.length > 1 ? onClear : undefined} | ||
onClick={onClick} | ||
active={activeModelUri?.uri.path === model.uri.path} | ||
onRename={onRename} | ||
/> | ||
))} | ||
<LemonButton onClick={() => onAdd()} icon={<IconPlus fontSize={14} />} /> | ||
|
@@ -34,19 +38,47 @@ interface QueryTabProps { | |
onClick: (model: QueryTab) => void | ||
onClear?: (model: QueryTab) => void | ||
active: boolean | ||
onRename: (model: QueryTab, newName: string) => void | ||
} | ||
|
||
function QueryTabComponent({ model, active, onClear, onClick }: QueryTabProps): JSX.Element { | ||
function QueryTabComponent({ model, active, onClear, onClick, onRename }: QueryTabProps): JSX.Element { | ||
const [tabName, setTabName] = useState(() => model.name || NEW_QUERY) | ||
const [isEditing, setIsEditing] = useState(false) | ||
|
||
const handleRename = (): void => { | ||
setIsEditing(false) | ||
onRename(model, tabName) | ||
} | ||
|
||
return ( | ||
<button | ||
<div | ||
onClick={() => onClick?.(model)} | ||
className={clsx( | ||
'space-y-px rounded-t p-1 flex flex-row items-center gap-1 hover:bg-[var(--bg-light)] cursor-pointer', | ||
active ? 'bg-[var(--bg-light)] border-t border-l border-r' : 'bg-bg-3000', | ||
onClear ? 'pl-3 pr-2' : 'px-3' | ||
)} | ||
> | ||
{model.view?.name ?? 'New query'} | ||
{isEditing ? ( | ||
<AutoTab | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: the editing mode is very blended with the existing styling. Not sure exactly best styling here but something slightly to indicate you're in editing mode besides just a blinking cursor would be helpful |
||
value={tabName} | ||
onChange={(e) => setTabName(e.target.value)} | ||
onBlur={handleRename} | ||
autoFocus | ||
handleRename={() => onRename(model, tabName)} | ||
onKeyDown={(e) => { | ||
if (e.key === 'Enter') { | ||
handleRename() | ||
} else if (e.key === 'Escape') { | ||
setIsEditing(false) | ||
} | ||
}} | ||
/> | ||
) : ( | ||
<div onDoubleClick={() => setIsEditing(!isEditing)} className="flex-grow text-left whitespace-pre"> | ||
{tabName} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll want to keep |
||
</div> | ||
)} | ||
{onClear && ( | ||
<LemonButton | ||
onClick={(e) => { | ||
|
@@ -57,6 +89,6 @@ function QueryTabComponent({ model, active, onClear, onClick }: QueryTabProps): | |
icon={<IconX />} | ||
/> | ||
)} | ||
</button> | ||
</div> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,9 +37,12 @@ export const editorModelsStateKey = (key: string | number): string => `${key}/ed | |
export const activeModelStateKey = (key: string | number): string => `${key}/activeModelUri` | ||
export const activeModelVariablesStateKey = (key: string | number): string => `${key}/activeModelVariables` | ||
|
||
export const NEW_QUERY = 'New query' | ||
|
||
export interface QueryTab { | ||
uri: Uri | ||
view?: DataWarehouseSavedQuery | ||
name: string | ||
} | ||
|
||
export const multitabEditorLogic = kea<multitabEditorLogicType>([ | ||
|
@@ -62,6 +65,7 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([ | |
updateState: true, | ||
runQuery: (queryOverride?: string, switchTab?: boolean) => ({ queryOverride, switchTab }), | ||
setActiveQuery: (query: string) => ({ query }), | ||
renameTab: (tab: QueryTab, newName: string) => ({ tab, newName }), | ||
setTabs: (tabs: QueryTab[]) => ({ tabs }), | ||
addTab: (tab: QueryTab) => ({ tab }), | ||
createTab: (query?: string, view?: DataWarehouseSavedQuery) => ({ query, view }), | ||
|
@@ -134,12 +138,10 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([ | |
[] as QueryTab[], | ||
{ | ||
addTab: (state, { tab }) => { | ||
const newTabs = [...state, tab] | ||
return newTabs | ||
return [...state, tab] | ||
}, | ||
removeTab: (state, { tab: tabToRemove }) => { | ||
const newModels = state.filter((tab) => tab.uri.toString() !== tabToRemove.uri.toString()) | ||
return newModels | ||
return state.filter((tab) => tab.uri.toString() !== tabToRemove.uri.toString()) | ||
}, | ||
setTabs: (_, { tabs }) => tabs, | ||
}, | ||
|
@@ -199,22 +201,38 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([ | |
actions.addTab({ | ||
uri, | ||
view, | ||
name: view?.name || NEW_QUERY, | ||
}) | ||
actions.selectTab({ | ||
uri, | ||
view, | ||
name: view?.name || NEW_QUERY, | ||
}) | ||
|
||
const queries = values.allTabs.map((tab) => { | ||
return { | ||
query: props.monaco?.editor.getModel(tab.uri)?.getValue() || '', | ||
path: tab.uri.path.split('/').pop(), | ||
view: uri.path === tab.uri.path ? view : tab.view, | ||
name: tab.name, | ||
} | ||
}) | ||
actions.setLocalState(editorModelsStateKey(props.key), JSON.stringify(queries)) | ||
} | ||
}, | ||
renameTab: ({ tab, newName }) => { | ||
const updatedTabs = values.allTabs.map((t) => { | ||
if (t.uri.toString() === tab.uri.toString()) { | ||
return { | ||
...t, | ||
name: newName, | ||
} | ||
} | ||
return t | ||
}) | ||
actions.setTabs(updatedTabs) | ||
actions.updateState() | ||
}, | ||
selectTab: ({ tab }) => { | ||
if (props.monaco) { | ||
const model = props.monaco.editor.getModel(tab.uri) | ||
|
@@ -254,6 +272,7 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([ | |
const allModelQueries = localStorage.getItem(editorModelsStateKey(props.key)) | ||
const activeModelUri = localStorage.getItem(activeModelStateKey(props.key)) | ||
const activeModelVariablesString = localStorage.getItem(activeModelVariablesStateKey(props.key)) | ||
|
||
const activeModelVariables = | ||
activeModelVariablesString && activeModelVariablesString != 'undefined' | ||
? JSON.parse(activeModelVariablesString) | ||
|
@@ -284,6 +303,7 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([ | |
newModels.push({ | ||
uri, | ||
view: model.view, | ||
name: model.name || NEW_QUERY, | ||
}) | ||
mountedCodeEditorLogic && initModel(newModel, mountedCodeEditorLogic) | ||
} | ||
|
@@ -315,10 +335,12 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([ | |
actions.selectTab({ | ||
uri, | ||
view: activeView, | ||
name: NEW_QUERY, | ||
}) | ||
} else if (newModels.length) { | ||
actions.selectTab({ | ||
uri: newModels[0].uri, | ||
name: NEW_QUERY, | ||
}) | ||
} | ||
} else { | ||
|
@@ -339,7 +361,7 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([ | |
return { | ||
query: props.monaco?.editor.getModel(model.uri)?.getValue() || '', | ||
path: model.uri.path.split('/').pop(), | ||
view: model.view, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a mis delete? We'll want the view to still be here otherwise the views won't persist between refreshes |
||
name: model.name || NEW_QUERY, | ||
} | ||
}) | ||
localStorage.setItem(editorModelsStateKey(props.key), JSON.stringify(queries)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transitioned to
<div />
, since we were throwing a nested button warning.