? Failure

User tests: Successful: Unsuccessful:

avatar Scrabble96
Scrabble96
4 Sep 2019

Pull Request for Issue #26123 .

Summary of Changes

Removed two lines of code (79 and 84) as IE11-related and not necessary.

Testing Instructions

Create com_content category override. Remove the two lines from blog.php
Create or view a category blog.

Expected result

The blog should display correctly

Actual result

Documentation Changes Required

avatar Scrabble96 Scrabble96 - open - 4 Sep 2019
avatar Scrabble96 Scrabble96 - change - 4 Sep 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Sep 2019
Category Front End com_content
avatar Scrabble96
Scrabble96 - comment - 4 Sep 2019

Ok. I'm a bit confused (as usual).

@Quy The original lines 79 to 84 were like this:
<div class="blog-item-content"><!-- Double divs required for IE11 grid fallback -->
<?php
$this->item = & $item;
echo $this->loadTemplate('item');
?>
</div>
(can't get the indentations to show here)

I removed the first and last line, so it should have looked like this for lines 79 to 82 inclusive:

<?php
$this->item = & $item;
echo $this->loadTemplate('item');
?>

I don't know why the opening and closing php tags are not lined up. They were when I edited the file in my repository.

@zero-24 You are asking me to change the single php code with two items into two separate php items like this:

<?php $this->item = & $item; ?>
<?php echo $this->loadTemplate('item'); ?>

PHP is definitely not my field of expertise, so could you explain why they need to be two separate instructions? Thanks.

avatar Scrabble96
Scrabble96 - comment - 4 Sep 2019

And what does 'Resolve conversation' mean?

avatar Scrabble96 Scrabble96 - change - 4 Sep 2019
Labels Added: ?
avatar Scrabble96
Scrabble96 - comment - 4 Sep 2019

Sorry (again). I'm looking at Github's instructions (https://help.github.com/en/articles/reverting-a-pull-request) but do not see 'Revert' anywhere:

It might be easiest just to close this one and start all over again.

avatar N6REJ
N6REJ - comment - 4 Sep 2019

image
is this right?

avatar brianteeman
brianteeman - comment - 4 Sep 2019

@N6REJ that is nothing to do with this PR. Please create your own issue instead of introducing new issues to an existing pr

avatar SharkyKZ
SharkyKZ - comment - 5 Sep 2019

There's some CSS associated with blog-item-content class

It will need to be updated.

avatar Scrabble96
Scrabble96 - comment - 5 Sep 2019

There's some CSS associated with blog-item-content class

It will need to be updated.

In that case, why not just leave the file as it was and only remove the IE11 comment from the file instead of the whole div?

avatar SharkyKZ
SharkyKZ - comment - 5 Sep 2019

If those nested divs were added only because IE needed them, it would be nice to have them removed. SCSS changes are pretty simple. Just need to move .blog-item-content rules to .blog-item.

avatar Scrabble96
Scrabble96 - comment - 6 Sep 2019

If those nested divs were added only because IE needed them, it would be nice to have them removed. SCSS changes are pretty simple. Just need to move .blog-item-content rules to .blog-item.

Ignore - commented incorrectly, so removed it.

avatar SharkyKZ
SharkyKZ - comment - 6 Sep 2019

Relevant SCSS code is linked in the comments above.

avatar Scrabble96
Scrabble96 - comment - 6 Sep 2019

Relevant SCSS code is linked in the comments above.

Sorry, yes, that's why I removed my comment. I couldn't see how to delete it.

avatar Scrabble96
Scrabble96 - comment - 6 Sep 2019

Ok. I've looked at the scss files, but as I have zero experience using scss I can't offer to change this code. There's some code for @media (max-width: 767.98px) that appears in the template.css but not in the above scss file, so that must be held somewhere else. If I attempt to change this code I'll probably screw it up.

avatar N6REJ
N6REJ - comment - 8 Sep 2019

@N6REJ that is nothing to do with this PR. Please create your own issue instead of introducing new issues to an existing pr

@brianteeman
Like usual you have to be a jerk and ignore the item... This is part of the file she's submitting so why would this NOT be the proper time to bring it up. STOP BEING A JERK!

avatar N6REJ
N6REJ - comment - 8 Sep 2019

@Scrabble96 @SharkyKZ I believe what he's saying is you could move 157-197

  flex-direction: column;
  .boxed & {
    background-color: #fff;
    box-shadow: 0 0 2px rgba(52, 58, 67, 0.1), 0 2px 5px rgba(52, 58, 67, 0.08), 0 5px 15px rgba(52, 58, 67, 0.08), inset 0 3px 0 $cassiopeia-template-color;
    .item-content {
      padding: 25px;
    }
  }
  .image-left &, .image-right & {
    flex-direction: row;
  }
  .item-image {
    margin-top: 3px;
    margin-bottom: 15px;
    overflow: hidden;
    .boxed & {
      margin-bottom: 0;
    }
    .image-right & {
      order: 1;
    }
    .image-bottom & {
      order: 1;
      margin-top: -15px;
    }
  }
  .item-content {
    .image-left & {
      padding-left: 25px;
    }
    .image-right & {
      padding-right: 25px;
    }
  }
  .image-left &, .image-right & {
    flex-direction: row;
    @include media-breakpoint-down(sm) {
      flex-direction: column;
    }
  }

and move it between 93 & 94 so that it would be this..


 .blog-item {
  padding: 0 ($cassiopeia-grid-gutter / 2) $cassiopeia-grid-gutter ($cassiopeia-grid-gutter / 2);
    display: flex;
  flex-direction: column;
  .boxed & {
    background-color: #fff;
    box-shadow: 0 0 2px rgba(52, 58, 67, 0.1), 0 2px 5px rgba(52, 58, 67, 0.08), 0 5px 15px rgba(52, 58, 67, 0.08), inset 0 3px 0 $cassiopeia-template-color;
    .item-content {
      padding: 25px;
    }
  }
  .image-left &, .image-right & {
    flex-direction: row;
  }
  .item-image {
    margin-top: 3px;
    margin-bottom: 15px;
    overflow: hidden;
    .boxed & {
      margin-bottom: 0;
    }
    .image-right & {
      order: 1;
    }
    .image-bottom & {
      order: 1;
      margin-top: -15px;
    }
  }
  .item-content {
    .image-left & {
      padding-left: 25px;
    }
    .image-right & {
      padding-right: 25px;
    }
  }
  .image-left &, .image-right & {
    flex-direction: row;
    @include media-breakpoint-down(sm) {
      flex-direction: column;
    }
  }
}

you could then remove the empty 

.blog-item-content {
}
avatar mbabker
mbabker - comment - 8 Sep 2019

@N6REJ Adding unrelated changes to a pull request complicates matters. Just because someone is changing a file does not give carte blanche to request every line in that file be reviewed or changed. Patches should focus on a singular change only.

avatar N6REJ
N6REJ - comment - 8 Sep 2019

@N6REJ Adding unrelated changes to a pull request complicates matters. Just because someone is changing a file does not give carte blanche to request every line in that file be reviewed or changed. Patches should focus on a singular change only.

@mbabker ty. As you know I would never presume I have carte blanche, but your response explained things better. Thanks

avatar N6REJ
N6REJ - comment - 8 Sep 2019

ugh, I always make a mess... @Scrabble96 hope that helps you.

avatar SharkyKZ
SharkyKZ - comment - 8 Sep 2019

@mbabker This is not an unrelated change. SCSS must be updated to account for markup changes in this PR. Otherwise the styling is broken.

avatar Scrabble96
Scrabble96 - comment - 8 Sep 2019

Thank you @N6REJ and @SharkyKZ
I'll have a look later today at adding that code. It makes sense as my PR fails otherwise.

avatar mbabker
mbabker - comment - 8 Sep 2019

Not the SCSS, the other question about the item var assignment.

avatar Scrabble96
Scrabble96 - comment - 8 Sep 2019

I'm not sure what I'm supposed to be doing, now. @N6REJ has made the SCSS changes into his own branch, but it appears to have failed checks, so if I tried the same thing, wouldn't it have the same result?

This PR is showing as 'All checks have passed' What happens next?.

avatar SharkyKZ
SharkyKZ - comment - 9 Sep 2019

Failed checks unrelated. If SCSS changes are not included in this PR then #26208 should be merged first before this can be tested. In the meantime could you remove the second instance of the same code and fix indentation?

<div class="blog-item-content"><!-- Double divs required for IE11 grid fallback -->

avatar Scrabble96
Scrabble96 - comment - 9 Sep 2019

In the meantime could you remove the second instance of the same code and fix indentation?

I've deleted a branch hope that's achieved what you want which is I think where the other instance of the changed file was. This repo thing is very, very confusing to a new user.
I've looked at the changed file on here and as far as I can see the paired div tags are lined up, so I don't know what else needs changing.

avatar joomla-cms-bot joomla-cms-bot - change - 9 Sep 2019
Category Front End com_content Front End com_content Templates (site)
avatar brianteeman
brianteeman - comment - 9 Sep 2019

@N6REJ No. Thats not what I said. You have to submit them as pr here https://github.com/Scrabble96/joomla-cms/tree/Scrabble96-patch-4

avatar N6REJ
N6REJ - comment - 9 Sep 2019

@brianteeman if that doesn't fix it I give up.. idk how to do anything else.

avatar ciar4n
ciar4n - comment - 9 Sep 2019

#26238 (comment)

If true then this would also need to be closed

avatar Scrabble96
Scrabble96 - comment - 10 Sep 2019

@N6REJ and @brianteeman I have no idea what's going on now. PRs have been submitted to my patch. What do I do with them?

If I click 'Resolve conflicts' it just highlights some code but doesn't say what the conflict is. I suspect that my original change to the SCSS has been outdated by @ciar4n's changes for mobile-first code in PR #26237.

avatar Scrabble96
Scrabble96 - comment - 11 Sep 2019

Ok. I'm going to close this. If someone else wants to start again they are welcome.

avatar Scrabble96 Scrabble96 - change - 11 Sep 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-09-11 10:53:17
Closed_By Scrabble96
avatar Scrabble96 Scrabble96 - close - 11 Sep 2019

Add a Comment

Login with GitHub to post a comment