[BasicUI] App icons no more rendered in snapshot 4228

[lolodomo]

It was still working in snapshot 4217

@florian-h05 : could it be due to the merge of #2606 ? I hope you tested the UI before merging.

[florian-h05]

I tested it but my testing was fairly limited.
Can you revert that commit locally and then build and try out?

[lolodomo]

It works again if I revert gulp upgrade.

[florian-h05]

Okay in that case I am sorry that this slipped through, let’s revert it then.

[florian-h05]

See #2713.

[lolodomo]

Just for my information is there a Git feature to simply revert a merged PR ?

[florian-h05]

Yeah, on the bottom of a merged PR you can find a button to easily revert it from the GitHub UI:

[lolodomo]

Ok, thank you a lot, I never saw that button !

[lolodomo]

I am now trying to investigate what was wrong.
With Basic UI compiled with gulp 5.0.0, I got these errors in browser console:

If I check the font files sizes created by gulp into folder src\main\resources\web\fonts, they are fully different:

• material-icons.woff:: 164 912 bytes with gulp 4.0.2 while 299 808 bytes with gulp 5.0.0
• material-icons.woff2 : 128 352 bytes with gulp 4.0.2 while (231 689 bytes with gulp 5.0.0

Gulp is copying these files from ./node_modules/material-icons/iconfont
The sizes in this folder remains the same. So the problem is the copy of files done by Gulp 5.0.0 which changes the file content !

Our Gulp code:

	var paths = {
	        FontLibs: [
			'./node_modules/material-icons/iconfont/material-icons.woff*',
			'./node_modules/framework7-icons/fonts/Framework7Icons-Regular.woff*',
			'./node_modules/framework7-icons/fonts/Framework7Icons-Regular.ttf'
	        ]
	    };
	
	gulp.task("copyFontLibs", function () {
	    return gulp.src(paths.FontLibs)
	        .pipe(gulp.dest('./src/main/resources/web/fonts'));
	});

[lolodomo]

gulpjs/gulp#2790

With Gulp v5, you need apparently to specify encoding: false to stream.

https://medium.com/gulpjs/announcing-gulp-v5-c67d077dbdb7

Stream encoding

Our streams now default to UTF-8 encoding. Previously, our streams took whatever data was emitted without considering any encoding. However, in this release, we resolved a 10 year old issue to support custom encodings and we default these to UTF-8. Most usage of gulp won’t need to change anything, but certain plugins may produce non-UTF-8 output and you’ll need to set { encoding: false } on your gulp streams.

[lolodomo]

Ok, with this updated code, font files are now unchanged:

	gulp.task("copyFontLibs", function () {
	    return gulp.src(paths.FontLibs, { encoding: false })
	        .pipe(gulp.dest('./src/main/resources/web/fonts'));
	});

Will have to check all Gulp tasks but I believe others are handling non binary files.