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.

Sign up for GitHub

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

Jump to bottom

Add support for including custom CSS, JS and metadata tags to a page #1203

Merged
merged 6 commits into from
Oct 13, 2020
Merged

Add support for including custom CSS, JS and metadata tags to a page #1203

merged 6 commits into from
Oct 13, 2020

Conversation

vladbailescu
Copy link
Member

Added support for including custom CSS, JS and metadata tags to a page

Fixes #1202

@codecov
Copy link

codecov bot commented Oct 8, 2020 •
edited
Loading

Codecov Report

Merging #1203 into development will increase coverage by 0.09%.
The diff coverage is 89.85%.

Impacted file tree graph

@@                Coverage Diff                @@
##             development    #1203      +/-   ##
=================================================
+ Coverage          83.88%   83.97%   +0.09%     
- Complexity          1591     1609      +18     
=================================================
  Files                150      152       +2     
  Lines               4505     4574      +69     
  Branches             742      755      +13     
=================================================
+ Hits                3779     3841      +62     
- Misses               288      290       +2     
- Partials             438      443       +5
Impacted Files Coverage Δ Complexity Δ
...omponents/internal/models/v2/HtmlPageItemImpl.java 83.33% <83.33%> (ø) 9.00 <9.00> (?)
...be/cq/wcm/core/components/models/HtmlPageItem.java 90.90% <90.90%> (ø) 3.00 <3.00> (?)
...m/core/components/internal/models/v2/PageImpl.java 83.33% <100.00%> (+3.33%) 32.00 <5.00> (+5.00)
.../com/adobe/cq/wcm/core/components/models/Page.java 100.00% <100.00%> (ø) 21.00 <1.00> (+1.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad1385b...89d3dde. Read the comment docs.

@vladbailescu vladbailescu linked an issue Oct 12, 2020 that may be closed by this pull request
Add support for including custom CSS, JS and metadata tags to a page #1202
Closed
@msagolj
Copy link
Contributor

msagolj commented Oct 12, 2020

This new feature should be documented in the README of the page component.

@vladbailescu
Copy link
Member Author

@msagolj , added README documentation in c4af1d3

@vladbailescu
Updated structure documentation
47cec50
@vladbailescu
* Renamed PageItems* to HtmlPageItems*
e5267f9 
* Moved items attributes to dedicated node
* Updated docs and test content
jckautzmann
jckautzmann reviewed Oct 13, 2020
View reviewed changes
bundles/core/src/main/java/com/adobe/cq/wcm/core/components/config/HtmlPageItemsConfig.java
~ See the License for the specific language governing permissions and
~ limitations under the License.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
package com.adobe.cq.wcm.core.components.config;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the HtmlPageItemsConfig interface be part of the com.adobe.cq.wcm.core.components.internal package, similar to the context aware configuration of the data layer?
The context aware config for the PDF Viewer component is also part of the internal package (com.adobe.cq.wcm.core.components.internal.services.pdfviewer).

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually think that was a bad choice and all configurations should be API, as we expect people to follow those structures. I was going to propose we find a way to migrate existing configuration to that package as well, in a backwards compatible way.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as we treat all context aware configurations similarly I'm fine. Let's keep this one as it is and move the other ones to the same public package later.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@vladbailescu vladbailescu merged commit 76b86d3 into development Oct 13, 2020
@vladbailescu vladbailescu deleted the issues/1202 branch October 13, 2020 12:58
msagolj pushed a commit that referenced this pull request Oct 22, 2020
@vladbailescu @msagolj
Add support for including custom CSS, JS and metadata tags to a page (#…
cd4756a 
…1203)

* Add support for including custom CSS, JS and metadata tags to a page
* Added documentation to component README
* Updated docs and test content
* Updated tests

Fixes #1202
@stefanseifert stefanseifert mentioned this pull request Nov 2, 2020
Closed
@csenavneet
Copy link

@vladbailescu: I am using wcm.io editor and when trying to create CA config using page items, I am only getting an option to provide prefix path. Is there any other configuration needed to configure items?

Attached is the screenshot for your reference.
Screen Shot 2021-01-15 at 2 07 17 PM

CC: @rajramo61 @rajeevyadav2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msagolj msagolj msagolj approved these changes

@jckautzmann jckautzmann jckautzmann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for including custom CSS, JS and metadata tags to a page
4 participants
@vladbailescu @msagolj @csenavneet @jckautzmann