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

fix(form/login/wizard): fixed undefined issues with IE11 script #1871

Merged
merged 5 commits into from May 30, 2019
Merged

fix(form/login/wizard): fixed undefined issues with IE11 script #1871

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d016645
fix(form/login/wizard): fixed undefined isseus with IE11 script
dlabaj May 30, 2019
f8ff830
fix(form/login/wizard): fixed undefined isseus with IE11 script
dlabaj May 30, 2019
6b7abc5
fix(wizard): moved nav link
dlabaj May 30, 2019
8590ce7
fix(wizard): updated based on review comments
dlabaj May 30, 2019
4cb5b0c
fix(wizard): moved nav link and nav link number
dlabaj May 30, 2019
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
8 changes: 4 additions & 4 deletions src/patternfly/components/Form/form.scss
View file
Open in desktop
Expand Up @@ -21,14 +21,14 @@
--pf-c-form__group--m-action--MarginTop: var(--pf-global--spacer--xl);

// actions
--pf-c-form__actions--MarginTop: calc(var(--pf-c-form__actions--child--MarginTop) * -1);
--pf-c-form__actions--MarginRight: calc(var(--pf-c-form__actions--child--MarginRight) * -1);
--pf-c-form__actions--MarginBottom: calc(var(--pf-c-form__actions--child--MarginBottom) * -1);
--pf-c-form__actions--MarginLeft: calc(var(--pf-c-form__actions--child--MarginLeft) * -1);
--pf-c-form__actions--child--MarginTop: var(--pf-global--spacer--sm);
--pf-c-form__actions--child--MarginRight: var(--pf-global--spacer--sm);
--pf-c-form__actions--child--MarginBottom: var(--pf-global--spacer--sm);
--pf-c-form__actions--child--MarginLeft: var(--pf-global--spacer--sm);
--pf-c-form__actions--MarginTop: calc(var(--pf-c-form__actions--child--MarginTop) * -1);
--pf-c-form__actions--MarginRight: calc(var(--pf-c-form__actions--child--MarginRight) * -1);
--pf-c-form__actions--MarginBottom: calc(var(--pf-c-form__actions--child--MarginBottom) * -1);
--pf-c-form__actions--MarginLeft: calc(var(--pf-c-form__actions--child--MarginLeft) * -1);

// Helpers
--pf-c-form__helper-text--MarginTop: var(--pf-global--spacer--xs);
Expand Down
2 changes: 0 additions & 2 deletions src/patternfly/components/Login/login.scss
View file
Open in desktop
Expand Up @@ -76,7 +76,6 @@
--pf-c-login__main-footer-links--PaddingRight: var(--pf-global--spacer--3xl);
--pf-c-login__main-footer-links--PaddingBottom: var(--pf-global--spacer--xl);
--pf-c-login__main-footer-links--PaddingLeft: var(--pf-global--spacer--3xl);
--pf-c-login__main-footer-links--MarginBottom: calc(var(--pf-c-login__main-footer-links--MarginBottom) * -1);
Copy link
Member

Choose a reason for hiding this comment

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

Same question as @mcoker had on removing --pf-c-wizard__toggle-list--MarginBottom from wizard.scss - did you mean to remove it completely or just move this line below --pf-c-login__main-footer-links--MarginBottom?

Copy link
Member

Choose a reason for hiding this comment

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

Wait, nevermind - I see this is actually self-referential?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. If you look now the margin there just computes to 0 since --pf-c-login__main-footer-links--MarginBottom isn't defined anywhere else.

Copy link
Member

Choose a reason for hiding this comment

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

As long as this wasn't a mistake where it really should have been assigned to another variable. I'm not sure why it would have been put in there that way. But, I don't know what the original specs were.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I looked at the commit where this was introduced (#1281) and looks like a mistake. The layout matches the design in sketch, too.

--pf-c-login__main-footer-links-item--PaddingRight: var(--pf-global--spacer--md);
--pf-c-login__main-footer-links-item--PaddingLeft: var(--pf-global--spacer--md);
--pf-c-login__main-footer-links-item--MarginBottom: var(--pf-global--spacer--sm);
Expand Down Expand Up @@ -238,7 +237,6 @@
flex-wrap: wrap;
justify-content: center;
padding: var(--pf-c-login__main-footer-links--PaddingTop) var(--pf-c-login__main-footer-links--PaddingRight) var(--pf-c-login__main-footer-links--PaddingBottom) var(--pf-c-login__main-footer-links--PaddingLeft);
margin-bottom: var(--pf-c-login__main-footer-links--MarginBottom);
}

.pf-c-login__main-footer-links-item {
Expand Down
10 changes: 5 additions & 5 deletions src/patternfly/components/Wizard/wizard.scss
View file
Open in desktop
Expand Up @@ -45,6 +45,11 @@
--pf-c-wizard__description--PaddingTop: var(--pf-global--spacer--sm);
--pf-c-wizard__description--Color: var(--pf-global--Color--light-200);

// Nav Link Before
--pf-c-wizard__nav-link--before--Width: 1.5rem;
--pf-c-wizard__nav-link--before--Height: 1.5rem;
--pf-c-wizard__nav-link--before--Top: 0;

// Toggle
--pf-c-wizard__toggle--BackgroundColor: var(--pf-global--BackgroundColor--100);
--pf-c-wizard__toggle--ZIndex: var(--pf-global--ZIndex--sm);
Expand All @@ -59,7 +64,6 @@

// Toggle list
--pf-c-wizard__toggle-list--MarginRight: var(--pf-global--spacer--sm);
--pf-c-wizard__toggle-list--MarginBottom: calc(var(--pf-c-wizard__toggle-list-item--MarginBottom) * -1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to remove this? Seems like you want to just move it below where --pf-c-wizard__toggle-list-item--MarginBottom is 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.

I did mean to remove it because this one also came out to be zero. I'll double check to make sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's -4px for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah my mistake thanks will add it back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


// Toggle list item
--pf-c-wizard__toggle-list-item--first-child--FontSize: var(--pf-global--FontSize--lg);
Expand Down Expand Up @@ -115,9 +119,6 @@
--pf-c-wizard__nav-link--m-disabled--Color: var(--pf-global--Color--dark-200);

// Nav link number
--pf-c-wizard__nav-link--before--Width: 1.5rem;
--pf-c-wizard__nav-link--before--Height: 1.5rem;
--pf-c-wizard__nav-link--before--Top: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of moving just these 3 lines, can you move this whole block up so that the // Nav link number CSS is all in one spot?

--pf-c-wizard__nav-link--before--BackgroundColor: var(--pf-global--BackgroundColor--300);
--pf-c-wizard__nav-link--before--BorderRadius: var(--pf-global--BorderRadius--lg);
--pf-c-wizard__nav-link--before--Color: var(--pf-global--Color--100);
Expand Down Expand Up @@ -270,7 +271,6 @@
flex-wrap: wrap;
align-items: baseline;
margin-right: var(--pf-c-wizard__toggle-list--MarginRight);
margin-bottom: var(--pf-c-wizard__toggle-list--MarginBottom);
Copy link
Contributor

Choose a reason for hiding this comment

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

Then add this back if you add --pf-c-wizard__toggle-list--MarginBottom back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

list-style: none;
}

Expand Down