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

DS-582 Add information about twig file name on schema section #2316

Merged
merged 3 commits into from
Sep 13, 2021

Conversation

adamszalapski
Copy link
Collaborator

@adamszalapski adamszalapski commented Sep 10, 2021

Jira

https://pegadigitalit.atlassian.net/browse/DS-582

Summary

Add information about the twig file name on the schema section.

Details

Applies to components with multiple schemas:

  • packages/components/bolt-listing-teaser
  • packages/components/bolt-navbar
  • packages/components/bolt-page-footer
  • packages/components/bolt-page-header
  • packages/components/bolt-side-nav
  • packages/layouts/bolt-holy-grail
  • packages/layouts/bolt-layout

How to test

Run this code locally and check if the twig file name is shown on components with multiple schemas and it's not visible on single schemas.

@github-actions github-actions bot added the type: feature List this PR in the 'Features' section of the release notes. label Sep 10, 2021
@colbytcook colbytcook temporarily deployed to feature/DS-582--branch-preview September 10, 2021 16:11 Inactive
Copy link
Collaborator

@danielamorse danielamorse left a comment

Choose a reason for hiding this comment

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

@adamszalapski code looks good 👍

Do you know why these changes aren't reflected on Accordion? I recall the logic of the schema template is kind of convoluted. Accordion might match some condition that these other components do not.

Page Header looks good. Navbar is showing incorrect Twig template names. Before we merge this, we should review each component with multiple schemas and fix the names in the schema where necessary. Let's discuss today in huddle.

@colbytcook
Copy link
Contributor

@danielamorse @adamszalapski the reason the Accordion and the Grid are not showing the twig names is because they are using the old version of including the nested schemas. The new version you list an array of schemas in the package.json, the older model you include those files directly in the schema file.

I am not sure if we should proceed to just update those two components to the new standard way or try to make this PR work with them. I think we should update those two components to the latest method.

Copy link
Collaborator

@danielamorse danielamorse left a comment

Choose a reason for hiding this comment

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

Checked again with @colbytcook's update. Verified all the components/layouts with multiple schemas match the filenames in the docs.

colbytcook
colbytcook approved these changes Sep 13, 2021
@danielamorse danielamorse changed the title Add information about twig file name on schema section DS-582 Add information about twig file name on schema section Sep 13, 2021
@colbytcook colbytcook had a problem deploying to feature/DS-582--branch-preview September 13, 2021 18:14 Failure
@colbytcook colbytcook merged commit 04901ed into master Sep 13, 2021
@colbytcook colbytcook deleted the feature/DS-582 branch September 13, 2021 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature List this PR in the 'Features' section of the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants