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.

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

Add a banner mixin, helping future releases and maintenance #36178

Merged
merged 3 commits into from Jul 11, 2022
Merged

Add a banner mixin, helping future releases and maintenance #36178

merged 3 commits into from Jul 11, 2022

Conversation

GeoSot
Copy link
Member

@GeoSot GeoSot commented Apr 15, 2022 •
edited

@XhmikosR , @mdo

It's just a proposal to help on future maintenance.

Feel free to close it if you think it doesn't help

ref: #35205 ???

mdo
mdo approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@mdo mdo left a comment •
edited

Choose a reason for hiding this comment

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

I dig it! Wonder if we can reduce to a single file though so we don't have to import a file and then include a mixin? Maybe just a _copyright.scss file? Dunno if we can pass any conditionals to that though.

alecpl
alecpl reviewed Apr 22, 2022
View reviewed changes
scss/mixins/_banner.scss Outdated Show resolved Hide resolved
@GeoSot GeoSot added the css label May 12, 2022
@GeoSot GeoSot added this to In progress in v5.2.0-stable via automation May 17, 2022
@GeoSot GeoSot force-pushed the gs/use-banner-mixin-in-scss branch from eeb0dd2 to 582c9b3 Compare May 18, 2022 08:33
@GeoSot GeoSot force-pushed the gs/use-banner-mixin-in-scss branch from 582c9b3 to 1161b17 Compare June 21, 2022 08:04
@GeoSot GeoSot force-pushed the gs/use-banner-mixin-in-scss branch from 1161b17 to 133a8ab Compare June 30, 2022 21:19
@GeoSot
Copy link
Member Author

GeoSot commented Jun 30, 2022

Dunno if we can pass any conditionals to that though.

Only if we define a standard variable before each import, but I don't think is a good practice 🤔

julien-deramond
julien-deramond reviewed Jul 5, 2022
View reviewed changes
scss/bootstrap-reboot.scss Outdated Show resolved Hide resolved
scss/mixins/_banner.scss Show resolved Hide resolved
scss/mixins/_banner.scss Outdated Show resolved Hide resolved
@GeoSot @julien-deramond
Update scss/bootstrap-reboot.scss
4c6a827 
Co-authored-by: Julien Déramond <julien.deramond@orange.com>
@julien-deramond julien-deramond self-requested a review July 6, 2022 09:28
v5.2.0-stable automation moved this from In progress to Reviewer approved Jul 6, 2022
mdo
mdo reviewed Jul 11, 2022
View reviewed changes
scss/mixins/_banner.scss Outdated Show resolved Hide resolved
scss/bootstrap-reboot.scss Outdated Show resolved Hide resolved
@mdo mdo marked this pull request as ready for review July 11, 2022 18:58
@mdo mdo requested a review from a team as a code owner July 11, 2022 18:58
@mdo mdo merged commit 54317bf into main Jul 11, 2022
v5.2.0-stable automation moved this from Reviewer approved to Done Jul 11, 2022
@mdo mdo deleted the gs/use-banner-mixin-in-scss branch July 11, 2022 23:36
@ntkme ntkme mentioned this pull request Jul 13, 2022
Closed
2 tasks
@ntkme ntkme mentioned this pull request Jul 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alecpl alecpl alecpl left review comments

@mdo mdo mdo approved these changes

@julien-deramond julien-deramond julien-deramond approved these changes

Assignees
No one assigned
Labels
css
Projects
No open projects
v5.2.0-stable
  
Done
v5.2.0-stable
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@GeoSot @mdo @alecpl @julien-deramond