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

FontAwesome 5.6.3 and updated versions of frameworks #14031

Merged
merged 26 commits into from Jan 8, 2019

Conversation

@alroniks
Copy link
Collaborator

@alroniks alroniks commented Aug 8, 2018

What does it do?

This PR provides updated tools for building default MODX manager theme to avoid various warnings messages during the build and also provides updated icons font FontAwesome to the latest free version - 5.2.0 (1295 icons against only 675 in 4.7)

Why is it needed?

This PR solves issues with outdated packages that were used for building the default theme, also allows using new classes .fa .fab .fas .far for icons in the manager.

Related issue(s)/PR(s)

Fixes #13849
#12790
#14030

All works fine, and old style of defining icons works also, but testing strongly needed.

@alroniks alroniks requested review from Mark-H and opengeek as code owners Aug 8, 2018
@alroniks alroniks changed the title 3x feature 13849 fa and updated libs [MODX 3] FontAwesome 5.2.0 and updated versions of frameworks Aug 8, 2018
@ilyautkin
Copy link
Contributor

@ilyautkin ilyautkin commented Aug 13, 2018

I tested this PR. New icons work. No bugs found

1ynbwo

Copy link
Collaborator

@Jako Jako left a comment

Just two notes …

@@ -1,25 +1,25 @@
{
"name": "revolution-theme-default",
"title": "MODX Revolution",
"version": "2.3.0",
"version": "2.7.0",

This comment has been minimized.

@Jako

Jako Aug 13, 2018
Collaborator

3.0.0?

This comment has been minimized.

@alroniks

alroniks Aug 13, 2018
Author Collaborator

Indeed. First version was prepared for 2.7

"grunt-growl": "^0.1.5",
"grunt-imageoptim": "^1.4.1",
"grunt-rename": "^0.1.4",
"grunt-sass": "^1.0.0"
},
"dependencies": {
"node-sass": "^4.8.3"
"node-sass": "^4.9.2"

This comment has been minimized.

@Jako

Jako Aug 13, 2018
Collaborator

Should that be in devDependencies, since we use it for development only?

@jonleverrier
Copy link
Contributor

@jonleverrier jonleverrier commented Aug 13, 2018

Just tested this PR. New icons work well!

The only small things I noticed whilst testing was, under elements tab;

  • Some of the icons seem bolder than usual
  • Some of the icons seem larger than before

shot1

In the new sidebar - some of the icons are now solid colour vs the outlined version before.

shot2

@alroniks
Copy link
Collaborator Author

@alroniks alroniks commented Aug 13, 2018

It's why I wrote that testing needed, but it's awaited. The whole font was changed.

@jonleverrier
Copy link
Contributor

@jonleverrier jonleverrier commented Aug 13, 2018

What's the best way to provide you feedback @alroniks ? I could make some small adjustments to the icons so they looked like they did before (as best as possible), whilst using the new Font Awesome.

@alroniks
Copy link
Collaborator Author

@alroniks alroniks commented Aug 14, 2018

@jonleverrier You can push adjustments directly to this PR. I guess it's the best way.

@Jako
Copy link
Collaborator

@Jako Jako commented Sep 19, 2018

The build part is changed different in #14091 (removed bower). This could be added here too.

Copy link
Collaborator

@Mark-H Mark-H left a comment

Has a minor merge conflict but looks good

@Mark-H Mark-H self-assigned this Dec 28, 2018
@Mark-H Mark-H removed their assignment Dec 28, 2018
@Mark-H
Copy link
Collaborator

@Mark-H Mark-H commented Dec 28, 2018

I tried to merge and resolve the conflicts, but the removal of bower in #14091 and this are conflicting heavily. Afraid this will need some more work... do you think you have time to pick it up again @alroniks?

@alroniks
Copy link
Collaborator Author

@alroniks alroniks commented Dec 29, 2018

@Mark-H As FA released already 5.6 version it makes sense to close this PR and create a new. I will do it.

@alroniks alroniks dismissed stale reviews from JoshuaLuckers and Mark-H via 2285893 Jan 2, 2019
@alroniks alroniks changed the title FontAwesome 5.2.0 and updated versions of frameworks FontAwesome 5.6.3 and updated versions of frameworks Jan 2, 2019
@alroniks
Copy link
Collaborator Author

@alroniks alroniks commented Jan 2, 2019

@Mark-H I've updated font awesome to 5.6.3 and fixed conflicts. Now it should work.

Copy link
Collaborator

@Jako Jako left a comment

The code still uses bower which was removed in #14091. I could help to remove that part, if you want.

Or am I wrong with that?

@Jako
Jako approved these changes Jan 3, 2019
@Mark-H
Mark-H approved these changes Jan 8, 2019
@alroniks alroniks self-assigned this Jan 8, 2019
@alroniks alroniks merged commit 1974b04 into modxcms:3.x Jan 8, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
alroniks added a commit that referenced this pull request Jan 8, 2019
* upstream/pr/14031: (23 commits)
  Fixed status icon
  Updated font files
  Update bourbon, neat and font awesome
  Fixed icons issue in breadcrumbs
  Fixed version and deps in package.json
  tidied up sidebar after new icons
  further adjustments to .x-tree-node-ct .x-tree-node .icon
  adjusted .x-tree-node-ct .x-tree-node .icon so the icons sit horizontally centered
  enhanced and removed duplicate styles for .tree-pseudoroot-node.x-tree-node-el > .icon
  added overide for .icon-refresh and .icon-plus-circle to make them slightly smaller
  Fixed some issues after cherry-picking
  Fixed missed brands font family
  Massive fixes of icons and styles related to icons
  Replace webfonts to fonts as it was before
  New icons and adopting styles to the new versions of framework
  New style of breakpoints
  New icons and adopted styles to new borubon/neat for navbar
  Updated icons for forms
  Updated styles for buttons, including updated icon vars
  Updated files of font
  ...
@alroniks alroniks deleted the alroniks:3x-feature-13849_fa_and_updated_libs branch Jan 8, 2019
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

6 participants