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

Embedded GitHub Gists change their order in an unpredictable way #14428

Closed
luvejo opened this issue Aug 6, 2021 · 14 comments · Fixed by #18289
Closed

Embedded GitHub Gists change their order in an unpredictable way #14428

luvejo opened this issue Aug 6, 2021 · 14 comments · Fixed by #18289
Assignees
Labels
area: media images, videos, podcasts, liquid tags, etc. bug smash Approved bugs for the DEV community bug smash bug always open for contribution external contributors welcome contribution is welcome!

Comments

@luvejo
Copy link

luvejo commented Aug 6, 2021

Problem

Sometimes, when I open one of my tutorials, I find the embedded gists have changed their order. For example, if I specify snippet-1.js, snippet-5.js gets rendered instead, and so on. They change their order in an unpredictable way.

Needless to say what this means for readers. Specially in a long tutorial.

Steps to reproduce

It happens sometimes, even if I open the post in incognito mode and no browser extensions. I just can't tell when.

Desktop:

  • Browser: Google Chrome 92.0.4515.131
  • Browser: Firefox 90.0.2 (64 bit)
  • OS: Ubuntu 21.04

Smartphone:

  • OS: Android
  • Browser: Google Chrome 92.0.4515.131

By the way, I haven't found another way to show sample code with line numbers. I tried to upload screenshots of my snippets, but tall images are scaled because of their max-height rule. That's why I opened this feature request.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2021

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss. The OSS Community Manager or the engineers on OSS rotation will follow up.

For full info on how to contribute, please check out our contributors guide.

@rhymes
Copy link
Contributor

rhymes commented Aug 6, 2021

Ah! This reminded me of something, so I went digging. Apparently #1679 didn't fix it as we thought it would, or it regressed in some way.

:-/

@rhymes rhymes added area: liquid tags bug always open for contribution labels Aug 6, 2021
@cmgorton cmgorton added bug smash Approved bugs for the DEV community bug smash external contributors welcome contribution is welcome! labels Aug 6, 2021
@vishaldeepak
Copy link
Contributor

someone trying to recreate the issue I have noticed that, simply refreshing the page does not cause the problem. Here how I got it reproduced

  • Goto This Turorial Page
  • On the right hit the author
  • In the author's page simply click on the same article found in the first page
  • Gists should now be mixed up.

Can I be assigned this, would take a shot

@nickytonline
Copy link
Contributor

Assigning this to you @vishaldeepak.

@vishaldeepak
Copy link
Contributor

@rhymes
The problem seems to be happening because of this code

  for (const gistTag of gistTags) {
    postscribe(gistTag, gistTag.firstElementChild.outerHTML, {
      beforeWrite(text) {
        return gistTag.childElementCount > 3 ? '' : text;
      },
    });
  } 

Looks like the HTML pages already have script tag on the page. The (postscibe pacakge)[https://github.com/krux/postscribe#how-to-use-postscribe-to-render-an-ad-after-load] suggests that we create script tags on the fly(not inserted in the dom).

A fix could be store span tags instead of script in the processed_html attributes of an Article model. Then in the js dynamically string replace span to script. This fixed the problem on my local

What do you think?

@nsbarsukov
Copy link

@vishaldeepak Hi!
I have faced with the same problem in my article 😢

snippetsBug.mov

What happens in this video:

  • Before page refresh => incorrect order of snippets
  • After page refresh => correct order of snippets

Almost a year has already passed (since the opening of the issue). Any updates ?(

@vishaldeepak
Copy link
Contributor

PR was merged, and should have solved the issue .

@nsbarsukov
Copy link

@benhalpern Hi!
Sorry, i dont know whom i should tag.
All previous participants of this issue removed themselves from core-team: here and here.

That is why I have mentioned you here as developer from core-team.

Could you help me with this issue, please ?
Unfortunately, this issue may be mistakenly considered as resolved. But it is not :(

@vishaldeepak
Copy link
Contributor

Do you have the steps on how to create this on your local, maybe I can help out, its been some time since Ive worked on this

@nsbarsukov
Copy link

Do you have the steps on how to create this on your local, maybe I can help out, its been some time since Ive worked on this

I sincerely appreciate your help!

Unfortunately, I don't know how to reproduce it locally (I dont even know how launch forem-app locally).
But you can use this article.
It contains a lot of github snippets. And sometimes this snippets are shown in incorrect order (see video above).

@natepelzel
Copy link
Contributor

Hello! I'd like to help out with this. Would that be okay?

@vishaldeepak
Copy link
Contributor

@natepelzel I have not started work on this, so its ok from me

@natepelzel
Copy link
Contributor

@vishaldeepak I have WIP PR that I'd like some feedback on. I was able to fix the issue but I wanted to make sure I went about it in an agreeable way

#18289

@vishaldeepak
Copy link
Contributor

@natepelzel Hey I think a core member will be able to help you out on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: media images, videos, podcasts, liquid tags, etc. bug smash Approved bugs for the DEV community bug smash bug always open for contribution external contributors welcome contribution is welcome!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants