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

Update control buttons #40977

Merged
merged 8 commits into from Jun 7, 2021
Merged

Update control buttons #40977

merged 8 commits into from Jun 7, 2021

Conversation

maddiedierker
Copy link
Contributor

@maddiedierker maddiedierker commented Jun 4, 2021

This updates the control buttons to match Figma and moves the 'continue' button to a temporary spot (it and the 'settings' button will be moved/re-styled next week):

Screen.Recording.2021-06-04.at.1.09.18.PM.mov

There's also some clean-up to:

  • move styles definition below component definition
  • move <InputPrompt/> into a separate file
  • refactor the isHorizontal prop in <JavalabButton/> to actually make the icon/text horizontally stacked

Links

@maddiedierker maddiedierker requested a review from a team June 4, 2021 20:13
@@ -8,15 +8,20 @@
padding: 0;
}

.cm-content, .cm-gutter {
.cm-content,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: most of the changes in this file are formatting changes from prettier

}}
>
<JavalabButton
text={isRunning ? 'Stop' : 'Run'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be internationalized? (same question for other strings)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they weren't in i18n before, so i left them as-is, but i'll add them to the javalab/locale file

all: {
fontSize: 16,
width: 140,
justifyContent: 'center',
Copy link
Contributor

Choose a reason for hiding this comment

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

The justify-center feels pretty odd. Would left make more sense?

@maddiedierker maddiedierker requested a review from a team as a code owner June 7, 2021 17:30
Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

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

LGTM!

@maddiedierker maddiedierker merged commit b4d9400 into staging Jun 7, 2021
@maddiedierker maddiedierker deleted the javalab-control-buttons branch June 7, 2021 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants