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

fix: faute de frappe corrigée pour faire fonctionner GP avec Leaflet et Angular #382

Merged
merged 9 commits into from
Aug 8, 2024

Conversation

RupertBarrow
Copy link
Contributor

Pull request checklist

Verifiez que votre Pull Request remplit les conditions suivantes :

  • Des tests ont été ajoutés pour les changements (corrections de bugs ou features)
  • De la documentation a été mise à jour ou ajoutée si nécessaire (corrections de bugs ou features)
  • Un build (npm run build) a été lancé localement et s'est correctement déroulé
  • Les exemples impactés par les modifications (npm run samples) ont été testés et validés localement
  • Les tests (npm run test) sont passés localement

De plus, l'application github:geoportal-third-party-integration-master/simple-map-leaflet-angularqui ne buildait pas build correctement maintenant.

Type de Pull request

Quel type de changement cette Pull Request introduit-elle :

  • Bugfix
  • Feature
  • Mise à jour du style du code (syntaxe, renommage de fonctions)
  • Refactoring (lisibilité/performance du code, sans changements fonctionnels)
  • Changement sur le processus de build
  • Contenu de la documentation
  • Autres (décrire ci-après) :

Quel est le comportement actuel (avant PR) :

geoportal-extensions-leaflet n'est pasz utilisable dans un context Leaflet + Angular, car le fichier Styles.js importe des CSS. C'est à cause de la faute de frappe décrite dans ISSUE #381 qui est corrigée par cette PR.

Numéro du ticket : #381

Quel est le nouveau comportement :

  • correction de la faute de frappe
  • le build génère donc un Styles.js qui est compatible avec Leaflet et Angular
  • le build de github:geoportal-third-party-integration-master/simple-map-leaflet-angular ne fonctionnait pas : c'est maintenant corrigé

Cette PR introduit-elle des breaking changes ?

  • Oui
  • Non

Autres informations

Copy link
Contributor

@elias75015 elias75015 left a comment

Choose a reason for hiding this comment

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

Bonjour,

merci pour votre PR.

Quelques changements pour que celle-ci fonctionne aussi en mode package npm. En effet, aucune release npm n'a été publiée pour le package replace-bundle-webpack-plugin.

"replace-bundle-webpack-plugin": "^1.0.0",

Remplacer par le fork corrigé de l'erreur de typo :
"replace-bundle-webpack-plugin-edited": "^1.0.0",

var ReplaceWebpackPlugin = require("replace-bundle-webpack-plugin");

var ReplaceWebpackPlugin = require("replace-bundle-webpack-plugin");

var ReplaceWebpackPlugin = require("replace-bundle-webpack-plugin");

https://github.com/IGNF/geoportal-extensions/blob/77c1c97672d48031f20e9b3b8ed148c9a525e9f6/build/webpack/webpack.config.openlayers.modules.js#L16C1-L16C69

Remplacer par :
var ReplaceWebpackPlugin = require("replace-bundle-webpack-plugin-edited");

Copy link
Contributor

@lowzonenose lowzonenose left a comment

Choose a reason for hiding this comment

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

On peut aussi héberger en local le plugin comme pour les plugins JsDocWebPackPlugin et HandlebarsPlugin (cf. ./build/scripts/webpackPlugins) afin d'éviter d'utiliser un fork. On aurait ainsi la maintenance.

Copy link
Contributor

@elias75015 elias75015 left a comment

Choose a reason for hiding this comment

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

OK pour moi, merci

@elias75015 elias75015 marked this pull request as ready for review August 8, 2024 16:25
@elias75015 elias75015 merged commit d8dba4e into IGNF:develop Aug 8, 2024
1 check passed
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.

4 participants