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.

Sign up for GitHub

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

Jump to bottom

chore: recorder button styling #19231

Merged
merged 10 commits into from
Dec 15, 2022
Merged

chore: recorder button styling #19231

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
dadd42d
chore: recorder button styling
debs-obrien Dec 2, 2022
b10db57
chore: fixes from review
debs-obrien Dec 15, 2022
860097c
Merge branch 'main' into recorder-styling
debs-obrien Dec 15, 2022
635fabd
chore: fix merge and remove unused
debs-obrien Dec 15, 2022
a1f9029
chore: remove unused an fix padding
debs-obrien Dec 15, 2022
475a2df
chore: add back style
debs-obrien Dec 15, 2022
bffe3e0
chore: improve padding
debs-obrien Dec 15, 2022
f6fcf31
chore: revert changes to styling
debs-obrien Dec 15, 2022
a9732ad
chore: revert styling
debs-obrien Dec 15, 2022
0bf5988
chore: add missing style
debs-obrien Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/recorder/src/recorder.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ body.dark-mode .recorder .toolbar-button.toggled.record {
.recorder .toolbar .cm-wrapper {
margin-left: 8px;
display: flex;
padding: 8px;
padding: 5px;
}

.recorder .toolbar .CodeMirror {
Expand Down
6 changes: 3 additions & 3 deletions packages/recorder/src/recorder.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export const Recorder: React.FC<RecorderProps> = ({
<Toolbar>
<ToolbarButton icon='record' title='Record' toggled={mode === 'recording'} onClick={() => {
window.dispatch({ event: 'setMode', params: { mode: mode === 'recording' ? 'none' : 'recording' } });
}}>Record</ToolbarButton>
}}><span>Record</span></ToolbarButton>
<ToolbarButton icon='files' title='Copy' disabled={!source || !source.text} onClick={() => {
copy(source.text);
}}></ToolbarButton>
Expand Down Expand Up @@ -136,9 +136,9 @@ export const Recorder: React.FC<RecorderProps> = ({
<SourceView text={source.text} language={source.language} highlight={source.highlight} revealLine={source.revealLine}></SourceView>
<div className='vbox'>
<Toolbar>
<ToolbarButton icon='microscope' title='Explore' toggled={mode === 'inspecting'} onClick={() => {
<ToolbarButton icon='microscope' title='Pick locator' toggled={mode === 'inspecting'} onClick={() => {
window.dispatch({ event: 'setMode', params: { mode: mode === 'inspecting' ? 'none' : 'inspecting' } }).catch(() => { });
}}>Explore</ToolbarButton>
}}><span>Pick locator</span></ToolbarButton>
<CodeMirrorWrapper text={locator} language={source.language} readOnly={false} focusOnChange={true} wrapLines={true} onChange={text => {
setLocator(text);
window.dispatch({ event: 'selectorUpdated', params: { selector: text, language: source.language } });
Expand Down
22 changes: 18 additions & 4 deletions packages/web/src/components/toolbarButton.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,25 @@

.toolbar-button {
flex: none;
border: none;
outline: none;
color: var(--vscode-sideBarTitle-foreground);
background: transparent;
padding: 4px;
margin-left: 10px;
cursor: pointer;
display: inline-flex;
align-items: center;
border-radius: 6px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you aligning the visual language with?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using the border radius of 6px as same as githubs buttons

border: 1px solid var(--vscode-dropdown-border);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these seems to be best color for border for both light and dark mode as inspired by github buttons

font-family: inherit;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you resetting from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

line-height: 20px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already do flex + align items, why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

white-space: nowrap;
vertical-align: middle;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not use this style in flex.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

cursor: pointer;
appearance: none;
user-select: none;
text-decoration: none;
text-align: center;
padding: 5px 8px;
font-size: 14px;
color: var(--vscode-sideBarTitle-foreground);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok removed except for padding which is needed

}

.toolbar-button:disabled {
Expand All @@ -39,3 +49,7 @@
.toolbar-button:not(:disabled):active {
background-color: var(--vscode-toolbar-activeBackground);
}

.toolbar-button > :not(:last-child) {
margin-right: 8px;
}
4 changes: 3 additions & 1 deletion packages/web/src/components/toolbarButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface ToolbarButtonProps {
icon: string,
disabled?: boolean,
toggled?: boolean,
type?: 'button' | 'submit' | 'reset',
onClick: () => void,
}

Expand All @@ -32,10 +33,11 @@ export const ToolbarButton: React.FC<React.PropsWithChildren<ToolbarButtonProps>
icon = '',
disabled = false,
toggled = false,
type = 'button',
onClick = () => {},
}) => {
let className = `toolbar-button ${icon}`;
if (toggled)
className += ' toggled';
return <button className={className} onClick={onClick} title={title} disabled={!!disabled}><span className={`codicon codicon-${icon}`}></span>{ children }</button>;
return <button className={className} type={type} onClick={onClick} title={title} disabled={!!disabled}><span className={`codicon codicon-${icon}`}></span>{ children }</button>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can toolbar button be a submit / reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right

};