Skip to content

Update bootswatch and bootstrap dependencies #1030

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

Conversation

abdounikarim
Copy link
Contributor

Hey guys,

I think i've finished this PR #1029

Here is what I did :

  1. Update dependencies :
  1. Replace dependencies :
  1. Update assets :
  • replace bootstrap-sass/assets/javascripts/bootstrap/transition.js by bootstrap/scss/transitions
  1. Remove unused dependencies :
  1. Update templates in templates directory (sorry if i miss something)

  2. CSS :
    well class not replaced (not existing in the new Flatly theme

  3. Generate new files for production with yarn build

Let me know if i miss something please, thank you 😉

@noniagriconomie
Copy link
Contributor

@abdounikarim do you have any before/after pics?
nice work btw!

@mazarini
Copy link

Hi,
There is something use with twig for bootstrap :

twig:
    form_themes: ['bootstrap_4_layout.html.twig']

Actually, demo use :

twig:
    form_themes:
        - 'form/layout.html.twig'
        - 'form/fields.html.twig'

I don't known if it(s nedd change.

@javiereguiluz
Copy link
Member

@abdounikarim thanks a lot for your contribution. All the steps you outlined look OK to me. However, when testing this pull request, I found that it looks very different from before. I expected some changes, because we're upgrading from Bootstrap 3 to 4 and the template is also upgraded ... but some things look wrong. Here are some before/after comparisons:

Comparison 1

Before

before-1

After

after-1

Comparison 2

Before

before-2

After

after-2

Comparison 3

Before

before-3

After

after-3

Comparison 4

Before

before-4

After

after-4

Comparison 5

Before

before-5

After

after-5

@abdounikarim
Copy link
Contributor Author

@javiereguiluz hello, as i was told you on Slack, the Flatly theme was updated and there are some differences. For example :

  • the h1 has now a font-size with 3em (before he has a font-size with 39px)
  • there is no longer well class
    i'm gonna correct this to look like the actual theme in Symfony Demo"

@abdounikarim
Copy link
Contributor Author

@javiereguiluz i think i just finished design corrections. Can you check and tell me please ?
Cheers 😉

@stof
Copy link
Member

stof commented Sep 23, 2019

* bootstrap-sass/assets/javascripts/bootstrap/transition.js by bootstrap/scss/transitions

replacing a JS file by a SCSS one ? Isn't that a mistake ?

@abdounikarim
Copy link
Contributor Author

* bootstrap-sass/assets/javascripts/bootstrap/transition.js by bootstrap/scss/transitions

replacing a JS file by a SCSS one ? Isn't that a mistake ?

hey @stof , i didn't find any transition.js file in the bootstrap directory.

@abdounikarim
Copy link
Contributor Author

@stof i think i'm good now. cheers 😉

@abdounikarim
Copy link
Contributor Author

@javiereguiluz are we good now ? or do you want any change ? 😉

@import "~bootswatch/flatly/bootswatch";
@import "~bootswatch/dist/flatly/variables";
@import "~bootstrap/scss/bootstrap";
@import "~bootstrap/scss/transitions";
Copy link
Member

Choose a reason for hiding this comment

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

isn't this already imported by the main bootstrap file ?

@nicolas-grekas nicolas-grekas changed the base branch from master to main November 19, 2020 12:36
@javiereguiluz
Copy link
Member

Closing as fixed in #1412 (and #1183).

@abdounikarim I'm really sorry this contribution wasn't merged. I apologize for it and I hope we can do better for future contributions from you. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants