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

styles and copy for custom policies #733

Merged
merged 9 commits into from
Apr 1, 2019
Merged

styles and copy for custom policies #733

merged 9 commits into from
Apr 1, 2019

Conversation

thomasmaas
Copy link
Member

@thomasmaas thomasmaas commented Mar 28, 2019 •
edited

closes https://issues.jboss.org/browse/THREESCALE-2053

Screenshot 2019-03-31 15 19 08

Screenshot 2019-03-31 15 19 41

Screenshot 2019-03-31 15 20 06

Screenshot 2019-03-31 15 20 40

todo:

  • replace delete input for a to fix styling (@didierofrivia )
  • investigate cursor jumping (click in schema and hit back space; backspace acts on line below. only tested in safari (which is not supported browser) (@didierofrivia )
  • feedback on copy

@codecov
Copy link

codecov bot commented Mar 29, 2019 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (master@5c1f243). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #733   +/-   ##
=========================================
  Coverage          ?   92.87%           
=========================================
  Files             ?     2390           
  Lines             ?    77814           
  Branches          ?        0           
=========================================
  Hits              ?    72273           
  Misses            ?     5541           
  Partials          ?        0
Impacted Files Coverage Δ
...ers/provider/admin/registry/policies_controller.rb 93.54% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c1f243...728879e. Read the comment docs.

@thomasmaas thomasmaas self-assigned this Mar 29, 2019
@thomasmaas thomasmaas added the javascript Pull requests that update Javascript code label Apr 1, 2019
guicassolato
guicassolato previously approved these changes Apr 1, 2019
View reviewed changes
damianpm
damianpm reviewed Apr 1, 2019
View reviewed changes
app/javascript/src/Policies/styles/policies.scss
@@ -8,6 +8,7 @@ $goColor: #4a90e2;
$doColor: #54b45d;
$dontColor: #ca4a4b;
$subtleColor: #ddd;
$error-color: #c00;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this vars be moved to _colors.scss ?

Copy link
Member Author

Choose a reason for hiding this comment

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

same as below

app/javascript/src/Policies/styles/policies.scss
&::after {
color: $subtleColor; //$label-color;
content: 'Preview';
font-size: 10rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we use line-height-times() for font size, margins, paddings, etc ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but those variables are not available here. We'd have to include them… Hello React.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah.. oops you're right

@damianpm
Copy link
Contributor

damianpm commented Apr 1, 2019

Why is package-lock.json commited ? If no changes to package.json it must be unnecessary

@didierofrivia didierofrivia force-pushed the THREESCALE-2053-styling-and-copywriting-custom-policies branch from db82ff1 to 4647648 Compare April 1, 2019 11:08
@didierofrivia didierofrivia force-pushed the THREESCALE-2053-styling-and-copywriting-custom-policies branch from 4647648 to 728879e Compare April 1, 2019 15:21
@didierofrivia didierofrivia changed the title [wip] styles and copy for custom policies styles and copy for custom policies Apr 1, 2019
@thomasmaas thomasmaas merged commit 77db9ee into master Apr 1, 2019
@thomasmaas thomasmaas deleted the THREESCALE-2053-styling-and-copywriting-custom-policies branch April 1, 2019 16:06
josemigallas
josemigallas reviewed Apr 1, 2019
View reviewed changes
app/javascript/src/Policies/styles/policies.scss
content: 'Preview';
font-size: 10rem;
left: 4.5rem;
opacity: .5;
Copy link
Contributor

Choose a reason for hiding this comment

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

We disallow floating decimals in our JS, I think it should do the same here.

Copy link
Member

Choose a reason for hiding this comment

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

I think here is an exception, since the opacity property can take a value from 0.0 - 1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@didierofrivia didierofrivia didierofrivia left review comments

@josemigallas josemigallas josemigallas left review comments

@damianpm damianpm damianpm left review comments

@guicassolato guicassolato guicassolato approved these changes

@macejmic macejmic macejmic approved these changes

Assignees

@thomasmaas thomasmaas

Labels
javascript Pull requests that update Javascript code
Projects
None yet
Milestone
No milestone
6 participants
@thomasmaas @damianpm @guicassolato @didierofrivia @macejmic @josemigallas