chore: recorder button styling #19231
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
debs-obrien
commented
Dec 2, 2022
debs-obrien
commented
- improved styling based on GitHub paddings but with VS Code colors
- added a span for buttons with text and icon to easily add padding
- added type for button for better accessibility
- improved padding of wrapper for explore text
| text-align: center; | ||
| padding: 5px 8px; | ||
| font-size: 14px; | ||
| color: var(--vscode-sideBarTitle-foreground); |
There was a problem hiding this comment.
ok removed except for padding which is needed
| display: inline-flex; | ||
| align-items: center; | ||
| border-radius: 6px; |
There was a problem hiding this comment.
What are you aligning the visual language with?
There was a problem hiding this comment.
using the border radius of 6px as same as githubs buttons
| display: inline-flex; | ||
| align-items: center; | ||
| border-radius: 6px; | ||
| border: 1px solid var(--vscode-dropdown-border); |
There was a problem hiding this comment.
these seems to be best color for border for both light and dark mode as inspired by github buttons
| display: inline-flex; | ||
| align-items: center; | ||
| border-radius: 6px; | ||
| border: 1px solid var(--vscode-dropdown-border); | ||
| font-family: inherit; |
There was a problem hiding this comment.
What are you resetting from?
| border-radius: 6px; | ||
| border: 1px solid var(--vscode-dropdown-border); | ||
| font-family: inherit; | ||
| line-height: 20px; |
There was a problem hiding this comment.
We already do flex + align items, why is this needed?
| font-family: inherit; | ||
| line-height: 20px; | ||
| white-space: nowrap; | ||
| vertical-align: middle; |
There was a problem hiding this comment.
You should not use this style in flex.
| 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>; |
There was a problem hiding this comment.
How can toolbar button be a submit / reset?
pavelfeldman
approved these changes
Dec 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.