Fourth incremental upgrade to SPFx v1.16.1 #1424
Conversation
NishkalankBezawada
changed the title
NishkalankBezawada
changed the title
NishkalankBezawada
changed the title
NishkalankBezawada
changed the title
|
Hi @Adam-it This project uses 
So far, I have upgraded it until SPFx v1.16.1 Thanks, |
There was a problem hiding this comment.
@NishkalankBezawada I had a quick glance at the PR.
I am wondering if it is ok we are commenting out so much code and changing the behavior of the sample 🤔?
Could we have a double check on some of the comments I left? 🙏
samples/react-command-print/src/extensions/print/PrintCommandSet.ts
Outdated
Show resolved
Hide resolved
| //this.getTemplates(); | ||
| // Get select item values | ||
| this.getItemContent(); | ||
| //this.getItemContent(); |
There was a problem hiding this comment.
is it actually ok we are commenting this out? won't we like change the behavior of the sample?
There was a problem hiding this comment.
I enhanced it by using, since the original sample version was giving showing spinner continously, so i had to adjust the states too
await Promise.all([this.getTemplates(), this.getItemContent()]);
Instead calling the methods as below,
public componentDidMount() {
// Validate and create Print Settings list --> added list definition to elements.xml for adding Print List Settings
// this.initializeSettings();
// Get templates
this.getTemplates();
// Get select item values
this.getItemContent();
}
There was a problem hiding this comment.
ok then we should remove this commented out code. If we improved it and confirmed that it works and it's better I see little sense in leaving commented out code in sample. Fro someone new to the sample it will just create more confusion and question why those comments are here and why there is alternative code but commented out
samples/react-command-print/src/extensions/print/util/list-helper.ts
Outdated
Show resolved
Hide resolved
|
Hey @Adam-it here you go, with the comments and resolutions. I send you back this PR for review :) I have checked it again by serving the extension, it works good.
Thanks, |
| //this.getTemplates(); | ||
| // Get select item values | ||
| this.getItemContent(); | ||
| //this.getItemContent(); |
There was a problem hiding this comment.
ok then we should remove this commented out code. If we improved it and confirmed that it works and it's better I see little sense in leaving commented out code in sample. Fro someone new to the sample it will just create more confusion and question why those comments are here and why there is alternative code but commented out
There was a problem hiding this comment.
@NishkalankBezawada Awesome work 👏👏👏👏
I managed to build and tested it.
I have only some small comments I will fixup myself before merging.
You Rock 🤩
What's in this Pull Request?
Tried to upgrade the SPFx v1.7.0 to SPFx v1.20.0, but this project seems to have dependencies which will not work with react v17. I would like to give a try to upgrade to spfx v1.11.0 since, later versions of spfx has a significant breaking changes too. This might take some time, but I suggest to upgrade to spfx v1.11.0 and then try to do an incremental upgrade.
So this PR contains first incremental SPFx upgrade to SPFx v1.111.0
Thanks,
Nishkalank