NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar N6REJ
N6REJ
7 Aug 2020

Pull Request for Issue # .

Summary of Changes

renames $whiteoffset to $white-offset to more consistently match templates.

Testing Instructions

code review

Actual result BEFORE applying this Pull Request

was spelled 2 different ways.

Expected result AFTER applying this Pull Request

Documentation Changes Required

avatar N6REJ N6REJ - open - 7 Aug 2020
avatar N6REJ N6REJ - change - 7 Aug 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Aug 2020
Category Administration Templates (admin) NPM Change
avatar brianteeman
brianteeman - comment - 7 Aug 2020

Surely you can just remove
white-offset: $white-offset,

and anyway dont you need to change this in modals.scss

  .btn {
    padding: 0 22px;
    margin-right: .5rem;
    font-size: 1rem;
    line-height: 2.3rem;
    color: var(--atum-text-dark);
    background: var(--white);
    **border-color: var(--whiteoffset);**
    box-shadow: 1px 1px 1px 0 rgba(0, 0, 0, .25);
  }

avatar N6REJ
N6REJ - comment - 7 Aug 2020

Surely you can just remove
white-offset: $white-offset,

and anyway dont you need to change this in modals.scss

  .btn {
    padding: 0 22px;
    margin-right: .5rem;
    font-size: 1rem;
    line-height: 2.3rem;
    color: var(--atum-text-dark);
    background: var(--white);
    **border-color: var(--whiteoffset);**
    box-shadow: 1px 1px 1px 0 rgba(0, 0, 0, .25);
  }

I thought about that too @brianteeman I wasn't sure if it was safe to do so as I'm not a scss guru, so I played it safe.

As for the second item it will need to be changed won't it? for some reason it didn't come up in the global search.

avatar richard67
richard67 - comment - 8 Aug 2020

So or so I'm not sure if a code review is sufficient for testing, or if it not would be better to define a real test.

Due to our sometimes complicated scss structures you never know if a change which seems to be easy in code might not cause any unwanted side effects.

avatar N6REJ N6REJ - change - 8 Aug 2020
Labels Added: NPM Resource Changed ?
avatar N6REJ
N6REJ - comment - 8 Aug 2020

@chmst can someone from your group validate this pr?

avatar richard67
richard67 - comment - 9 Aug 2020

@chmst can someone from your group validate this pr?

Which group do you mean? The frontend template team to pimp up Cassiopeia? If so: What relation does it have to this PR here which changes scss for the backend template? Or what other group do you mean if not that one? Accessibility team? I also don't see a relation to that. So I am maximum confused now.

avatar N6REJ
N6REJ - comment - 9 Aug 2020

@richard67 SHOULDN'T affect the front-end but was thinking maybe someone on the cassiopeia team could verify the scss is done right.

avatar N6REJ N6REJ - change - 22 Aug 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-08-22 17:39:06
Closed_By N6REJ
avatar N6REJ N6REJ - close - 22 Aug 2020

Add a Comment

Login with GitHub to post a comment