staltz / hyperpunk

A cyberpunk theme for HyperTerm
MIT License
128 stars 23 forks source link

Glow effect no longer works in Hyperterm 2 #9

Open jcklpe opened 6 years ago

jcklpe commented 6 years ago

I've just installed Hyper 2 and love your theme but it seems that with the update the glow effect and background animation have stopped. That's from your theme right? I thought it might be hyper-oldschool but it's not displaying that behavior either.

staltz commented 6 years ago

Want commit rights to this repo? :)

jcklpe commented 6 years ago

@staltz I'd love to try and fix it myself but I don't actually know how. I'm a designer learning how to code and my skills aren't really up to that level yet, though I sure wish they were!

jcklpe commented 6 years ago

@staltz So I think I maybe found the problem. I found a place where the glow effect shows up! On bolded text that has been bolded by the "bold active tab" plugin. Which makes me think maybe the reason other stuff isn't glowy is because it's not bolded? Would my zsh syntax highlighting make stuff not glowy? Are only bolded things glowy?

Even with the glowy stuff the background does not change. Switching around from a couple of different extensions I've noticed some weird behavior for the backgrounds, and I think it's something to do with Hyper 2 since hyper-transparent-bg mentions on their repo 16 days ago just now supporting Hyper 2. So if you forked your background stuff from some body else, maybe your code has the same kinds of updates needed as theirs?

jcklpe commented 6 years ago

Hey stalz I hope I'm not bothering you. I know that fixing this isn't important but fiddling around with these plugins is part of how I'm trying to learn to code. Seeing other people code and figuring how and why they did stuff.

I took a closer look at what their changes were.

Looks like they changed this:

css: 'body { background: url(file:///exampleImageWeWontResolve.png) center; }\nbody { background: url(file:///exampleImageWeWontResolve.png) center; }'

To this:

 backgroundColor: 'transparent',
+    css: `
+      body { background: url(file:///exampleImageWeWontResolve.png) center; }
+      .hyper_main {
+        background: url(file:///exampleImageWeWontResolve.png) center;
+        background-size: cover;
+      }
+      .terms_terms {
+        background-color: transparent;
+      }
+    `

Which looks similar to what you have in your noise.js files. Though I can see you've actually serialized and included the pngs etc. But that's basically the difference between the two right? I'm... not entirely sure how to fix this myself. I can make a guess of what the problem is but I don't really know my JS syntax very well.

jcklpe commented 6 years ago

HAHA! WOOO! I FIGURED SOME OF IT OUT! I got the texture working on the tab pane now! WOO!

jcklpe commented 6 years ago

Bingo! I figured it out @staltz !!!💥💥💥💥💥💥💥💥💥💥💥💥💥💥💥💥

WOO! If you will let me do a pull request this could be my first pull request to get accepted on Github ever!

jcklpe commented 6 years ago

Uh... no, actually I didn't. It looks like that the texture still works on the tabs regardless of the changes I made or not :( Dagnabit. I thought I'd actually fixed something.

Basically I changed all the instances of background-image into background. Wasn't enough though.

I also tried changing this._term.scrollPort_.document_.body.appendChild(this._globalStyle); to this._term.scrollPort_.document_.hyper_main.appendChild(this._globalStyle); but really all this is just me guessing and trying to pattern match. I really need to sit down and finish my class in typscript and javascript etc. I know this is super simple. I can read the logic basically and I know CSS really well, I just can't seem to put the syntax together in my head.

I seem to have found the specific commit that made the background etc stuff fall away and that is basically Zeit's migration to styled-jsx.

This was another post I had open as a tab that I was talking stuff out in comment form like rubber ducky style. I'm including it here as documentation for why this issue occurred in part, or at least how I got to the solution

staltz commented 6 years ago

@thedonquixotic I'm happy to cheerlead here for you, you're not bothering me. :)

I don't have excess brain power to dig into the specifics, but I would guess since much changed in Hyper 2, as well as React having evolved, it's probably better to carefully rebuild this from scratch then to try to tweak it until it works. Following the Hyper 2 docs on how to build plugins might help.

jcklpe commented 6 years ago

@staltz I could be totally wrong but I think my change is an actual fix to the problem and that you don't need to delve any deeper into the specifics of Hyper 2. I was wrong about what was initially wrong about what was causing the issue though it helped me find the solution.

As I underst it, you are using JS to dynamically inject CSS into the page constantly which means that it doesn't follow the old or new style of styling components. The only thing I really had to change was to update the new css class names. They no longer style to body they style to .hyper_main. But from what I can tell it works!

This doesn't fix the glowing problem, but it's hard for me to tell what's supposed to glow and what's not, and how much of that might be due to my zsh settings overriding bolding of some text or something. I'm looking into that some more once I get my zsh configs etc finalized.

jcklpe commented 6 years ago

Oh ha, no wonder you didn't see my fix. I didn't finish completing the pull request yesterday. https://github.com/staltz/hyperpunk/pull/10

jcklpe commented 6 years ago

Been checking around and I can confirm that the glow effect is not working and this is due to the shift from DOM based hterm to the canvas based xterm.js for Hyper 2. Which makes at least the current implementation of css injection for the theme impossible.

The background is fixed though! Don't know how to fix the rest but we got what we got so far :)

SamyBencherif commented 4 years ago

Hi @jcklpe ! HyperPunk2.0 is looking good 😎 , but there is at least one residual issue before it can be the successor of the original HyperPunk.

Could you open up issues on the repo so I can submit a bug report?

Cheers, -S

jcklpe commented 4 years ago

@SamyBencherif Whoops, sorry! fire away :)