Skip to content

Conversation

@C-Lodder
Copy link
Member

Summary of Changes

Use double colon for pseudo elements

Testing Instructions

Code review

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Aug 30, 2019
@dgrammatiko
Copy link
Contributor

I should be blamed for this...

@brianteeman
Copy link
Contributor

it is actually optional for this pseudo element but I am happy to blame @dgrammatiko

@ghost ghost changed the title [4.0] Use double colon for psuedo elements [4.0] Use double colon for pseudo elements Aug 30, 2019
@C-Lodder
Copy link
Member Author

C-Lodder commented Aug 30, 2019

@brianteeman Still optional but better to distinguish between a pseudo element and pseudo class. You also never know if browsers will drop single colons for pseudo elements in the future.

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 15035c7


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26088.

1 similar comment
@Quy
Copy link
Contributor

Quy commented Aug 31, 2019

I have tested this item ✅ successfully on 15035c7


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26088.

@Quy Quy removed NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Aug 31, 2019
@Quy
Copy link
Contributor

Quy commented Aug 31, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26088.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 31, 2019
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Aug 31, 2019
@wilsonge wilsonge merged commit 30e93da into joomla:4.0-dev Sep 2, 2019
@wilsonge
Copy link
Contributor

wilsonge commented Sep 2, 2019

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 2, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Sep 2, 2019
@C-Lodder C-Lodder deleted the patch-2 branch September 2, 2019 10:45
@ghazal
Copy link
Contributor

ghazal commented Sep 2, 2019

I have tested this item 🔴 unsuccessfully on f24c441

Just tested this on a fresh install with Composer and NPM, and I got this error in the Terminal:

       The selector ".fa-dot-circle::before" was not found.
       Use "@extend .fa-dot-circle::before !optional" if the extend should be able to fail.
        on line 22 of build/media_source/system/scss/_icomoon.scss
>>     @extend .#{$fa-css-prefix}-dot-circle::before; 

compile-css could not proceed.<hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="https://issues.joomla.org/tracker/joomla-cms/26088">issues.joomla.org/tracker/joomla-cms/26088</a>.</sub>

wilsonge added a commit that referenced this pull request Sep 2, 2019
@wilsonge
Copy link
Contributor

wilsonge commented Sep 2, 2019

Thanks!

Fixed with 4a5a2fb (clearly no one actually tested)

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

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants