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

Upgrade bootswatch to version 4 #1183

Open
wants to merge 4 commits into
base: main
from
Open

Conversation

@bechir
Copy link

@bechir bechir commented Dec 24, 2020

Hi,

After comparing changes before and after in #1030 I said to myself It's maybe better to create remake the job with new newly updated Symfony 5.2 (and bootswtach v5.4.3).

I know @javiereguiluz wanted to resolve the conflicts himself but you can still see the work I did here

Comparaison home

Before

home

After

home

Comparaison blog

Before

blog

After

blog

Comparaison show code

Before

show-code

After

show-code

Comparaison pagination

Before

pagination

After

pagination

Comparaison login

Before

login

After

login

Comparaison admin

Before

admin

After

admin

Comparaison edit post

Before

edit

After

edit

Comparaison show post

Before

show-post

After

show-post

Comparaison edit post (tags)

Before

post-tags

After

post-tags

Comparaison post with errors

Before

post-errors

After

post-errors

CHANGELOG

In packages.json

  • Upgraded bootswatch to version 4.5.3
  • Upgraded bootstrap to version 4.5.3
  • Removed eonasdan-bootstrap-datetimepicker dependency (This is not compatible with bootstrap 4)

In scss files

  • Removed the $icon-font-path we no longer need this
  • Moved variables import to new _variables.scss file containg our custom variables
  • Updated imports
  • Replaced hardcoded colors value with custom variables name

In javascript files

  • Replaced deprecated method submit() by trigger('submit')
  • Updated highlightjs imports

In twig files

  • Replaced well class by jumbotron with some changes in the default paddings
  • Replaced label label-* by badge badge-*
  • Replaced pull-right by float-right I d'ont kown if it's the same but we have the same result
  • Updated classes name
  • Added the active route class to active

Tests

  • Beacause the form layout changed, we need to update assertions in App\Tests\Controller\Admin\BlogControllerTest::testAdminNewPost

I didn't added datetimepicker yet because the tempusdominus/bootstrap-4 project is getting rolled back into the orginal repo, but it's still in progress.

This lib is for bootstrap 4 and it's based on the Eonasdan's Bootstrap 3 date/time picker.
But flatpickr is more powerful with zero dependencies

I didn't added it yet, I'm juste waiting for feedbacks.

Thanks for your time!

bechir added 3 commits Dec 23, 2020
assets/js/app.js Outdated Show resolved Hide resolved
{% block form_errors -%}
{% if errors|length > 0 -%}
{% if form is not rootform %}<span class="help-block">{% else %}<div class="alert alert-danger">{% endif %}
<ul class="list-unstyled">
{%- for error in errors -%}
{# use font-awesome icon library #}
{# use font-awesome icon library # }

This comment has been minimized.

@noniagriconomie

noniagriconomie Dec 29, 2020
Contributor

to rollback

This comment has been minimized.

@bechir

bechir Dec 30, 2020
Author

Ok i rolled it back and i got this
Screen Shot 2020-12-30 at 1 21 09 AM

This comment has been minimized.

@bechir

bechir Dec 30, 2020
Author

And the test testAdminNewDuplicatedPost method is failing because we no longer have form-group.has-error

This comment has been minimized.

@noniagriconomie

noniagriconomie Dec 30, 2020
Contributor

i was commenting to rollback the extra space on the precise line i commented on

from

{# use font-awesome icon library #}

to
{# use font-awesome icon library # }

sorry if it was unclear :s

This comment has been minimized.

@bechir

bechir Dec 30, 2020
Author

Ah sorry.
The reason why I added that extra space is when I don't do so the code after that line will not be commented

@@ -3,6 +3,9 @@
common elements and decorates all the other templates.
See https://symfony.com/doc/current/templates.html#template-inheritance-and-layouts
#}

{% set route = app.request.get('_route') %}

This comment has been minimized.

@noniagriconomie

noniagriconomie Dec 29, 2020
Contributor

Suggested change
{% set route = app.request.get('_route') %}
{% set _route = app.request.get('_route') %}

what about this?

Copy link
Contributor

@noniagriconomie noniagriconomie left a comment

nice one :)

@bechir
Copy link
Author

@bechir bechir commented Dec 30, 2020

Hi @noniagriconomie,
Thanks you for your feedbacks

I updated my code but the datetime picker initialization is still missing.
You can check flatpickr the design look good and it's about 13KB

@noniagriconomie
Copy link
Contributor

@noniagriconomie noniagriconomie commented Dec 30, 2020

@bechir i think i am not the person who will decide if or not to replace this lib,
for me it does not matter, as the main point of this projetc is showcasing symfony etc, but lets wait main maintainers 👍

@bechir
Copy link
Author

@bechir bechir commented Dec 30, 2020

OK thanks you again.

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.