Complete Element Replacement Option

[vincenttaglia]

I am working on a front end development environment that uses this plugin to easily split up views that you are working on into partials, but with the CSS utility framework I'm using, making the partial a child element of the location breaks and complicates things.

This change introduces an option (replaceElement), which when used, replaces the location element instead of making the partial a child element.

From what I can tell, this is the first option. Thanks for making it so easy to add one!

[colbyfayock]

thanks for putting this together

can you add a new example / test that captures the use case?

[vincenttaglia]

Thanks for making a great piece of software I could put this together with!

Going to work on the test next.

When it comes to the example for the readme, how should I approach placement of listing the option? Create a new table specifically for the options object under the settings table?

[vincenttaglia]

I added a sub-section to settings called "Settings => Options". The sub-section includes info for the replaceElement option.

Also added test.

[colbyfayock]

this looks great @vincenttaglia nice job

my only question now is im wondering if it makes more sense to include replaceElement as a top level configuration rather than inside of options. sorry didn't think of this before, but given options is only being used at the moment for local variables (the name might need to be changed in the future to be more logical 🤔 ) and i feel like replaceElement might fit better along side the other properties like localtion and priority. thoughts?

if we got that route, i think it should still be relatively straightforward to add it, i think we would just need to destructure it here:

module.exports.injectPartial = function(base_html, { options, html, priority, location, replaceElement }) {

another thought - would it make sense to use priority and set priority: 'replace'? 🤔

[vincenttaglia]

Yea everything you are saying here makes complete sense. Part of the reason I put it there was that I didn't realize it was actually being used for local variables at first.

And I like the idea of making it a priority choice. It makes sense there I think.

I'll change that up.

On Wed, Dec 2, 2020, 7:26 AM Colby Fayock notifications@github.com wrote:

this looks great @vincenttaglia https://github.com/vincenttaglia nice job

my only question now is im wondering if it makes more sense to include replaceElement as a top level configuration rather than inside of options. sorry didn't think of this before, but given options is only being used at the moment for local variables (the name might need to be changed in the future to be more logical 🤔 ) and i feel like replaceElement might fit better along side the other properties like localtion and priority. thoughts?

if we got that route, i think it should still be relatively straightforward to add it, i think we would just need to destructure it here:

module.exports.injectPartial = function(base_html, { options, html, priority, location, replaceElement }) {

another thought - would it make sense to use priority and set priority: 'replace'? 🤔

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/colbyfayock/html-webpack-partials-plugin/pull/37#issuecomment-737228639, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADUIQHEPRK7FDPL54YEGDXDSSY57BANCNFSM4UFVJRIA .

[colbyfayock]

hey @vincenttaglia just wanted to check in on this :) and just a heads up my plan was to work through this with you before getting to your other pull request as i'd have some similar feedback over there (with tests and such) but would be good to knock this out first

[vincenttaglia]

Got busy this last week. Gonna be finishing it up out either tonight or tomorrow.

And sounds good!

On Thu, Dec 10, 2020 at 7:49 AM Colby Fayock notifications@github.com wrote:

hey @vincenttaglia https://github.com/vincenttaglia just wanted to check in on this :) and just a heads up my plan was to work through this with you before getting to your other pull request as i'd have some similar feedback over there (with tests and such) but would be good to knock this out first

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/colbyfayock/html-webpack-partials-plugin/pull/37#issuecomment-742531976, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADUIQHAU4Q4T2OLZCUOAHM3SUDGWXANCNFSM4UFVJRIA .

[vincenttaglia]

Ok I think that is everything.

I tried to edit docs to reflect the change, but not sure if I worded it the best. Feel free to change that.

Also, might want to take a second look at that regex. I made sure to include wildcards for characters between elements and in the opening tag, but I'm not sure how it might work with some edge cases. Although it shouldn't be too much of a problem since anyone that wants the element to be replaced should just be using something like <element></element> with no classes or anything between the tags.

Regex:

<element(.*?)>(.*?)<\/element>

[colbyfayock]

nice job @vincenttaglia thanks for all the effort here

[colbyfayock]

This should now be published in 0.8.0

@all-contributors please add @vincenttaglia for code and docs

[allcontributors[bot]]

@colbyfayock

I've put up a pull request to add @vincenttaglia! :tada: