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

Normalization status for story SDL #1

Open
truett-software opened this issue Dec 24, 2023 · 29 comments
Open

Normalization status for story SDL #1

truett-software opened this issue Dec 24, 2023 · 29 comments

Comments

@truett-software
Copy link
Owner

For your kind info,

We have completed the normalization for SDL story and player

Please check the below URL

File folder - https://github.com/truett-software/story-structured-data-linter/tree/test

Story:

Instances - https://truett-software.github.io/story-structured-data-linter/docs/story/

Player:

Instances - https://truett-software.github.io/story-structured-data-linter/docs/player/

And we are working on the normalization for next stories. Once complete will let you know
@jaygray0919

This comment was marked as resolved.

Sign in to view
@jaygray0919

This comment was marked as resolved.

Sign in to view
@jaygray0919
Copy link

jaygray0919 commented Dec 24, 2023 •
edited
Loading

These HTML pages are handled correctly:

https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-50/index.html

https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-55/index.html

Please emulate this approach for failed items.

@jaygray0919

This comment was marked as resolved.

Sign in to view
@jaygray0919

This comment was marked as resolved.

Sign in to view
@jaygray0919

This comment was marked as resolved.

Sign in to view
@jaygray0919

This comment was marked as resolved.

Sign in to view
@truett-software
Copy link
Owner Author

There is a problem with this CSS class:

 .ht-590 {
    height: 590px;
}

it is corrupting the layout of this image https://truett-software.github.io/story-structured-data-linter/docs/story/img/item-28/img-w970-h1059.webp i don't see the need for that CSS class

If you determine a need for a container to control an image, you must specify width and height. The ratio must be the same ratio as the image attributes. For example, for image-w900-h600, the container that reduces the size of the image by 80% is: (900 x .8) (600 x .8) or width="720" and height="480".

you might be able to use these expressions on the container CSS: width="80%" height="80" (which works well for inline SVG)

We checked the layout for overflow images and resized the width and height of the image, but it didn't fit in the container. So we reduced the size of the image container by 80% and now it fits in the grid container. And we have implemented this method for all overflow images.

In gist and one or two other images use these CSS class
.ht-590 {
height: 590px;
}

@truett-software
Copy link
Owner Author

This item fails to load correctly: https://github.com/truett-software/story-structured-data-linter/tree/test/docs/story/html/item-64

Is this installed? https://amp.dev/documentation/components/amp-google-document-embed

In amp-google-document-embed, We have used this script link in the story

<script async custom-element="amp-google-document-embed" src="https://cdn.ampproject.org/v0/amp-google-document-embed-0.1.js"></script>

Please check the below line

https://github.com/truett-software/story-structured-data-linter/blob/d0f5c1d20e9483ac097e0260a4a2314a3bd2f956/docs/story/index.html#L23

And we are checking the above issues. once fixed this issue will let you know.

@jaygray0919

This comment was marked as resolved.

Sign in to view
@jaygray0919
Copy link

The shared D3 items fail to load. Other D3 items load correctly.

@jaygray0919

This comment was marked as resolved.

Sign in to view
@truett-software
Copy link
Owner Author

For your kind info,

We have checked this google viewer loading issue

If we will call like this

./html/item-59/Patel-Schneider-SHACL.pdf

it shows loading issue.

but, we will call that pdf file in GitHub instances like this

https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-59/Patel-Schneider-SHACL.pdf

it work correctly.

Please check the below code

https://github.com/truett-software/story-structured-data-linter/blob/888de3afbe487daf96d2414695dfb163f748e56a/docs/story/index.html#L5662

And check this instances also

https://truett-software.github.io/story-structured-data-linter/docs/story/index.html#page=item-59

kindly let me know we will do this approach for all other slides. Once you confirm we will proceed.

@jaygray0919
Copy link

I now see that there are two file types that we need to manage outside of GitHub:

  • fonts
  • documents used by google doc preview

Your current update meets our normalization objectives.
Later, we'll create a AWS/S3 library of docs for google-doc-viewer
But, for normalization, the link above is good.

@truett-software
Copy link
Owner Author

this CSS continues to corrupt the images

.ht-590 {
  height: 590px;
}

i don't think it is needed. what problem is it solving?

i've looked at this again. I do not see any reason to use the above CSS class. The responsive property appears to properly scale an <amp-img> for different displays. Please explain the reason why we need this CSS class - you may be trying to solve a problem that I do not yet understand.

For amp-gist,

<amp-gist
            data-gistid="17c33c63e803a01612088b1a5b87ce64"
            layout="fixed-height"
            height="590">
</amp-gist>

We added height="590". But, it doesn't take this height and automatically takes the default height. So we set that CSS class to manage the height of the gist.

We have removed that CSS class in item-23 slide gist. Please check the code and instances below

Code link:
https://github.com/truett-software/story-structured-data-linter/blob/4f5b2b5bb5105738f4a673d50b780a99bf86d5a5/docs/story/index.html#L3147C10-L3147C10

Instance link:
https://truett-software.github.io/story-structured-data-linter/docs/story/index.html#page=item-23

Also please check the other gist slide (item-35) which are using that CSS class

Code link:
https://github.com/truett-software/story-structured-data-linter/blob/4f5b2b5bb5105738f4a673d50b780a99bf86d5a5/docs/story/index.html#L3979

Instance link:
https://truett-software.github.io/story-structured-data-linter/docs/story/index.html#page=item-35

We use that CSS class only in amp-gist and removed that CSS class in all other slides.

@truett-software
Copy link
Owner Author

There is a problem here: docs/story/html/_shared-items-28-33 Outlink returns a 404 i'll investigate why the target HTML files fail to load

Outlink returns a 404 error - Example 2-31 to Example 2-36

We have checked this _shared-items-28-33 folder files are not working in Outlink it returns 404 error. So we moved that folder html files to below way (/html/item-#)

These HTML pages are handled correctly:

https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-50/index.html

https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-55/index.html

Please emulate this approach for failed items.

We created an item-# folder in the /html folder and moved the specified item-id files into that folder.

GitHub link:
https://github.com/truett-software/story-structured-data-linter/tree/test/docs/story/html/item-28

Instance link:
https://truett-software.github.io/story-structured-data-linter/docs/story/index.html#page=item-28

Now its working. We have implemented this method for Example 2-31 slide to Example 2-36 slide.

But please check this outlink urls below

Outlink URL:
https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-28/circles.html
https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-29/sunburst.html
https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-30/sunburst-zoom-labels.html
https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-31/collapsible-boxes.html
https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-32/radial.html
https://truett-software.github.io/story-structured-data-linter/docs/story/html/item-33/treemap.html

These files display only the headers and the contents is empty.

@jaygray0919
Copy link

There is a problem with the links.
Here is the source:

The directory should be the same for each link. The only difference is the .html file
All .html files share one data source:

You can see the problem in chrome-dev

@jaygray0919
Copy link

OK, i see your point about the Gist and failing to render height=value
That looks like a bug. I'll file an issue with Google
However, I propose to leave it AS-IS and not resort to custom CSS.
Reasoning: "it works", and we want to use as-few-as-possible work arounds (item-specific CSS)
Our shared goal is maximum simplicity with the fewest possible variations
While you tried to correct an error, I just feel we should leave it "as is" and wait for Google to correct the error.
When google corrects the runtime, the instance will render the height correctly.

@truett-software
Copy link
Owner Author

truett-software commented Dec 29, 2023 via email

@truett-software
Copy link
Owner Author

There is a problem with the links. Here is the source:

The directory should be the same for each link. The only difference is the .html file All .html files share one data source:

You can see the problem in chrome-dev

We have analyzed problem with the below links

https://github.com/truett-software/story-structured-data-linter/tree/test/docs/story/html/_shared-items-28-33

So, we added flare.json file in item-28 to item-33 folder.

Now its working. Please check the below URL

GitHub link:

Instance link:

@truett-software
Copy link
Owner Author

OK, i see your point about the Gist and failing to render height=value That looks like a bug. I'll file an issue with Google However, I propose to leave it AS-IS and not resort to custom CSS. Reasoning: "it works", and we want to use as-few-as-possible work arounds (item-specific CSS) Our shared goal is maximum simplicity with the fewest possible variations While you tried to correct an error, I just feel we should leave it "as is" and wait for Google to correct the error. When google corrects the runtime, the instance will render the height correctly.

We have removed the ht-590 CSS class in story styles and not using that CSS in amp-gist.

@jaygray0919
Copy link

jaygray0919 commented Jan 3, 2024 •
edited
Loading

OK, I see what you did here: #1 (comment)

That is not what I designed, but we'll leave it AS-IS for this instance.
The purpose of this D3 example is to demonstrate how to use a single data instances (flare.json) for multiple view (different D3 graph types)
For that reason, all D3 graph types and the shared data are in one folder.
All you have to do is link a specific graph type (from the shared folder) on a specific page

@jaygray0919
Copy link

Here is how it should work:

one data source

@jaygray0919
Copy link

Also, please check this:

@truett-software
Copy link
Owner Author

OK, I see what you did here: #1 (comment)

That is not what I designed, but we'll leave it AS-IS for this instance. The purpose of this D3 example is to demonstrate how to use a single data instances (flare.json) for multiple view (different D3 graph types) For that reason, all D3 graph types and the shared data are in one folder. All you have to do is link a specific graph type (from the shared folder) on a specific page

We have checked the shared folder _shared-items-28-33 link in GitHub. But its not running it gives 404 page error.

For our checking, we have moved that file structure in our domain server, it works. please check the below URL:

Files:

Json file:

But the same structure doesn't work in GitHub structure.

@truett-software
Copy link
Owner Author

Also, please check this:

We have checked the amp-test url

1. Using <amp-google-document-embed> in <amp-story-grid-layer> it shows this below error

The tag 'amp-google-document-embed' may not appear as a descendant of tag 'amp-story-grid-layer'

calling amp-google-document-embed tag into amp-story-grid-layer is not supported. so it shows error.

To fix this issue:

  • we can use <amp-iframe>
  • we can call that pdf as an image <amp-img>

2. @media is called inside :root. So it shows error, we fixed this issue and updated. We have mentioned the code below

:root {
  --spacing-xxs: 0.5rem;
  --spacing-xs: 1.0rem;
  --spacing-s: 1.5rem;
  --spacing-m: 5.0rem;
  --spacing-l: 10rem;
  --spacing-xl: 15rem;
}
@media (min-width: 700px) {
  :root {
    --spacing-xxs: 1.0rem;
    --spacing-xs: 1.5rem;
    --spacing-s: 2.0rem;
    --spacing-m: 6.0rem;
    --spacing-l: 12.5rem;
    --spacing-xl: 20rem;
  }
}
@media (min-width: 1400px) {
  :root {
    --spacing-xxs: 1.25rem;
    --spacing-xs: 1.75rem;
    --spacing-s: 2.5rem;
    --spacing-m: 7.0rem;
    --spacing-l: 17.5rem;
    --spacing-xl: 25rem;
  }
}

@jaygray0919
Copy link

OK, now I remember about this issue.
Google does not allow amp-google-document-embed in the grid.
It is allowed in amp-attachment as you have implemented in Sowa.
Our solution follows the invalid amp-iframe solution: use amp-img and WEBP as a preview; put the target file on the outlink.

@jaygray0919
Copy link

re

To fix this issue:

we can use <amp-iframe>

Invalid. amp-iframe is not allowed in Story. I asked Google for a waiver but was denied for good reasons: ampproject/amphtml#39708

@truett-software
Copy link
Owner Author

OK, now I remember about this issue. Google does not allow amp-google-document-embed in the grid. It is allowed in amp-attachment as you have implemented in Sowa. Our solution follows the invalid amp-iframe solution: use amp-img and WEBP as a preview; put the target file on the outlink.

As per your comments

We have changed the amp-google-document-embed tag to amp img and target files are called in outlink.

Please check the below link

Instance link - https://truett-software.github.io/story-structured-data-linter/docs/story/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jaygray0919 @truett-software