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

New command: spo page header remove #6541

Open
mkm17 opened this issue Dec 29, 2024 · 14 comments
Open

New command: spo page header remove #6541

mkm17 opened this issue Dec 29, 2024 · 14 comments

Comments

@mkm17
Copy link
Contributor

mkm17 commented Dec 29, 2024

Usage

m365 spo page header remove

Description

Removes a modern page header

Options

Option Description
-n, --pageName <pageName> Name of the page to remove the header for.
-u, --webUrl <webUrl> URL of the site where the page to update is located.
-f, --force Don't prompt for confirmation.

Examples

Remove the page header

m365 spo page header remove --webUrl https://contoso.sharepoint.com/sites/team-a --pageName home.aspx

Default properties

No response

Additional Info

Currently, we can remove a header from the modern page. From what I observed in the internal structure of the page, it is defined as a banner web part in the full-width section.
The idea is to remove the first section of a page if the full-width section contains a banner web part.

@waldekmastykarz
Copy link
Member

Good suggestion. Since it's a remove command, let's add a -f, --force option to suppress confirmation prompt.

@mkm17
Copy link
Contributor Author

mkm17 commented Dec 31, 2024

@waldekmastykarz I have added the option to the description. I can take this one as well :)

@milanholemans
Copy link
Contributor

Hi @mkm17, is there a difference between this command and spo page header set --type none?

@mkm17
Copy link
Contributor Author

mkm17 commented Jan 1, 2025

Hi @milanholemans as I can see --type none sets a header without an image ->
image

@Adam-it
Copy link
Member

Adam-it commented Jan 2, 2025

@milanholemans, @waldekmastykarz, @mkm17 sorry for coming late to the party, due to my short break but I have a few comments/questions here.

This may be a bit tricky command and very specific and I will try to explain why:
The page header is nothing else than a section on the page which is full-width type and has a banner webpart in it. Is the idea to check this combination (so full width section with banner webpart) to determine if this is the page header and if so only then remove this section? Also we may just have a full-width section without a banner webpart in it which does not make sense I guess but it is possible to create this.
When we check (list) the sections the only way to determine if the section is full-width type and not some other type is that it has a single column with factor set as 0
image

This is a bit tricky. If this would change in future by MS this command will just break as our condition to get the page header will just not work anymore.
So if we check the above what we aim to create here is a very specific command for a very specific case. It will only allow to remove the first section of the page and only if it has a single column with factor set as 0 and a banner webpart in it.

I propose we should look at a bit more reusable feature.
I propose we add the spo page section remove command which currently we do not have 😮. We have the get, list, add but we do not have the remove command. This command will allow to remove any section on the page not only the first one under some specific conditions. Then I propose in the command examples we should add an example how to go about removing the header which will be the first section of the page. We could even add a small script sample to our repo which would present on how to remove the header from the page which would first list all the sections to check if the first one is full-width with banner webpart and only then remove this section.
Having this kind of commands and guidance how to adapt it for a specific case (like removing the header) provides more value and adds less code to maintain (which we already have a lot 🙂)

Let me know what do you think.

@mkm17
Copy link
Contributor Author

mkm17 commented Jan 2, 2025

@Adam-it ok, adding the spo page section remove command is a good idea. However, the same condition I wanted to use in this command (to find first full-width section with banner webpart) could also be applied to #6542 spo page header set.

Instead of creating a separate command to remove the header, we could update the existing header set command to allow setting the type to 'None' (as @milanholemans thought it works) and change the current None type to NoImage.

I also agree that we can make the page part of the solution more reusable. As mentioned in this comment, it seems that some page commands use the clientsidepage.ts and Page.ts files, while others have their own separate implementations for handling page content.

Ok, so the plan is:

  • Create spo page section remove specification and implement it
    and then
    (option 1) create a sample demonstrating how to remove a page header using spo page section remove
    (option 2) create a new command spo page header remove
    (option 3) update the existing spo page header set command to handle None header type

@milanholemans
Copy link
Contributor

I wonder what spo page header set is doing, this command should check for the factor property as well, no? I think it will always be difficult and tricky to manage pages since they change so often. That's also probably why @mkm17 has logged a few issues already, there are probably more (once again thank you for logging these issues).
Instead of checking for the full-width section, can't we just try to get the first section, check the first web part, and check if the web part GUID matches the GUID of a banner web part? Web part IDs should be something relatively fixed. Is that something that could work?

@mkm17, I'm afraid that updating spo page header set --type none will be a breaking change, since it's now intended to just update the page header to an imageless header. Therefore, I think creating spo page header remove will be clearer for folks.

@Adam-it
Copy link
Member

Adam-it commented Jan 2, 2025

Instead of checking for the full-width section, can't we just try to get the first section, check the first web part, and check if the web part GUID matches the GUID of a banner web part? Web part IDs should be something relatively fixed. Is that something that could work?

@milanholemans it is possible to have a full-width without a banner webpart. It does not make sense but it is possible. You may remove the webpart and save the page with empty section. In this case we should also somehow allow to remove it using our command(s).. that's why it is tricky

@mkm17, I'm afraid that updating spo page header set --type none will be a breaking change, since it's now intended to just update the page header to an imageless header.

yes it will be a breaking change. Lets not do it. Also it does not seem clear to me as well.

I also agree that we can make the page part of the solution more reusable. As mentioned in this #6540 (comment), it seems that some page commands use the clientsidepage.ts and Page.ts files, while others have their own separate implementations for handling page content.

@mkm17 yes this seems that is something that was overlooked from out end. If possible we could refactor and fix it up as well

Ok, so the plan is:

Create spo page section remove specification and implement it
and then
(option 1) create a sample demonstrating how to remove a page header using spo page section remove
(option 2) create a new command spo page header remove
(option 3) update the existing spo page header set command to handle None header type

@mkm17 yes I think we should create a spo page section remove command as I wrote it is more reusable and generic command which will allow to do more than just removing the header which is quite specific section of the page.
Then I would either go with option 1 or 2, for sure NOT option 3.
IMO a script sample demonstrating how to do that using the spo page section remove command is easier and provides less maintenance in the future as we may just update the script sample whenever we want.
Option 2 is also ok with spo page header remove command but IMO it is just a very specific case of spo page section remove command which will just create more code to handle and as @milanholemans mentioned pages are a total pain and change a lot and will change in future as well so there will be more changes we need to anticipate.

@pnp/cli-for-microsoft-365-maintainers any other feed?
@mkm17 what do you think?

@milanholemans
Copy link
Contributor

@milanholemans it is possible to have a full-width without a banner webpart. It does not make sense but it is possible. You may remove the webpart and save the page with empty section. In this case we should also somehow allow to remove it using our command(s).. that's why it is tricky

Should we really remove an empty section? You cannot see it on the page anyway, and if the section is empty, is actually a section, and not a page header, right?
One thing I didn't think of is that my approach is not 100% waterproof since you can add a banner web part to a section without it being a full-width section. Annoying!

@Adam-it
Copy link
Member

Adam-it commented Jan 3, 2025

Should we really remove an empty section? You cannot see it on the page anyway, and if the section is empty, is actually a section, and not a page header, right? One thing I didn't think of is that my approach is not 100% waterproof since you can add a banner web part to a section without it being a full-width section. Annoying!

that's why I am saying it is tricky and very specific case. and that is exactly why I propose we rather develop spo page section remove command which will be more generic command and provide guidance how to go about removing a page header using that command in either an example or a script sample

@milanholemans
Copy link
Contributor

that's why I am saying it is tricky and very specific case. and that is exactly why I propose we rather develop spo page section remove command which will be more generic command and provide guidance how to go about removing a page header using that command in either an example or a script sample

In my opinion, that would also mean that we should get rid of the spo page header set command. However it's still a useful command.

@mkm17
Copy link
Contributor Author

mkm17 commented Jan 3, 2025

@Adam-it @milanholemans

Hmm, are there any statistics on how often certain samples are viewed? Maybe we can check if there is interest in this type of action and, after some time, decide whether to turn it into a proper command.

If the new commands spo page section remove and the changes to spo page header set are implemented, the new command would be a combination of these two: using the condition to find the first section from spo page header set and the removal functionality from spo page section remove.

@milanholemans
Copy link
Contributor

Hmm, are there any statistics on how often certain samples are viewed? Maybe we can check if there is interest in this type of action and, after some time, decide whether to turn it into a proper command.

Yes, we can check the analytics of our docs to see how many times a particular page has been viewed.

@Adam-it
Copy link
Member

Adam-it commented Jan 4, 2025

Ok so it seems we have a plan right?
@mkm17 do you want to update the issue to align with what we agreed?
Then let's recheck it final time and open it up 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants