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

Removed background-image from list of supported CSS properties. #6816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

willholland
Copy link

Category

  • Content fix
  • New article

Related issues

  • No related issues

What's in this Pull Request?

Removed background-image from the list of CSS properties supported with custom column formatting.

This background-image CSS property requires the use of the '(' character, which is prohibited from use in the formatting schema and results in an error when used.

@VesaJuvonen
Copy link
Contributor

Docs Build status updates of commit 05cd53a:

❌ Validation status: errors

Please follow instructions here which may help to resolve issue.

File Status Preview URL Details
❌Error Details

  • [Error-RuningBuildFailed] Some unexpected errors happened when running build, please open a ticket in https://SiteHelp and include the error report for our team to troubleshoot

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

@thechriskent
Copy link
Collaborator

Technically, background-image is a whitelisted style property so the documentation isn't wrong. Practically, however, I agree that it's pretty useless to set it to initial or inherit (since all other uses are blocked because of how parenthesis in values are handled).

I wonder if a better approach might be to add an asterisk to the property and a note under the list describing the issue with parenthesis in values. (Basically everything but rgba() is blocked)

@andrewconnell andrewconnell added area:list-formatting Category: View, row & column formatting with JSON pr:do-not-merge Pull request not ready to be merged pr:in-review Actively reviewing pull request status:to-be-reviewed Issue needs to be reviewed by Microsoft for additional follow up / review. labels Mar 16, 2021
@willholland
Copy link
Author

I'd be happy to make whatever updates the team thinks best, so just let me know how I can help.

@willholland
Copy link
Author

@VesaJuvonen - any feedback on how we can improve this?

@StfBauer
Copy link
Collaborator

StfBauer commented Feb 22, 2024

Technically, background-image is a whitelisted style property so the documentation isn't wrong. Practically, however, I agree that it's pretty useless to set it to initial or inherit (since all other uses are blocked because of how parenthesis in values are handled).

I wonder if a better approach might be to add an asterisk to the property and a note under the list describing the issue with parenthesis in values. (Basically everything but rgba() is blocked)

Sorry for playing devils advocate here but technically many those properties can have a '(' in their values. To make it dependant which CSS property is allowed and which not by these characters is might not be the best approache forward.

@thechriskent
Copy link
Collaborator

Technically, background-image is a whitelisted style property so the documentation isn't wrong. Practically, however, I agree that it's pretty useless to set it to initial or inherit (since all other uses are blocked because of how parenthesis in values are handled).
I wonder if a better approach might be to add an asterisk to the property and a note under the list describing the issue with parenthesis in values. (Basically everything but rgba() is blocked)

Sorry for playing devils advocate here but technically many those properties can have a '(' in their values. To make it dependant which CSS property is allowed and which not by these characters is might not be the best approache forward.

We're not in disagreement. I think it's terrible how restricted the CSS values are. Basically, the values are checked for specific use cases like rgba( and translate( and all other uses of ( will invalidate your style value.

Since this is a discussion of the docs, my suggestion was to at least note the issue on the docs to avoid surprising users. My real ask would be to adjust the silly regex they're using to allow for more valid values but that is something the Microsoft team would have to do whereas the docs are something the community can help with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:list-formatting Category: View, row & column formatting with JSON pr:do-not-merge Pull request not ready to be merged pr:in-review Actively reviewing pull request status:to-be-reviewed Issue needs to be reviewed by Microsoft for additional follow up / review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants