Skip to content
This repository has been archived by the owner on May 7, 2023. It is now read-only.

Reinstate old UI and website link #151

Closed
atomiks opened this issue Aug 29, 2019 · 30 comments
Closed

Reinstate old UI and website link #151

atomiks opened this issue Aug 29, 2019 · 30 comments

Comments

@atomiks
Copy link
Contributor

atomiks commented Aug 29, 2019

@Chalarangelo as far as I'm aware you've recently converted many 30s projects over to new domains and a new starter UI kit. I apologise for being AFK for a few months so I understand you couldn't ask before it transitioned over.

I do like this new UI, however, I think parts of it feel like a beta still and think it could use more polish.

Some big problems I see:

  • The link that's first on Google (and was shared everywhere) is now broken
  • JavaScript snippets don't seem to work anymore
  • Some overflow problems with elements in the snippets
  • Some snippets don't gel properly with night mode (as it wasn't accounted for originally) and are hard to see
  • Clicking a link then clicking back doesn't restore scroll position

Is it possible to reinstate the old UI and old domain temporarily until these particular kinks are ironed out?

@Chalarangelo
Copy link
Owner

@atomiks, not really. But you could help iron out issues.

  • The old link will fix itself, it's a transition on Google's side. This would happen anyways, so I think it's as good a time as any. But I will see if we can use it as a redirect to fix the issue asap.
  • Js snippets should be an easy fix, we can look into that one together.
  • Point me to specific snippets with overflow problems, we should try and fix them together as soon as possible.
  • The scroll thing is a bit trickier actually, I haven't figured that one out, but I have a few ideas.

Basically if you can pr a fix for the js and overflow problems, I will take care of the rest. We are planning a launch next week to go hand in hand with the start of the new school year, so we should be able to figure it all out till then with your help. I will focus on these issues tomorrow as very high priority. @fejes713 could you help out a little bit?

@Chalarangelo
Copy link
Owner

For a more detailed explanation as to why not: The Travis builds were failing, GitHub pages had vulnerabilities and a lot of the websites couldn't be updated properly nor would they work with SEO. The point was to get more consistent while retaining individual identity of each repo. After having made these changes we can do a ton more with the new website and we need to remedy issues as we come across them. I definitely appreciate the feedback, but the plan is to move forward not back so we should try and fix problems quickly and once and for all.

The proposal is to use the old link from GitHub pages as a redirect to the website via netlify or something similar, which I will do tomorrow and also ask Google to redirect it (I think I know how). Fixing the issues on the site should be easy enough and the scrolling thing could be fixed with some simple redux state control when an element gets clicked, which I can also do tomorrow.

@Chalarangelo
Copy link
Owner

Chalarangelo commented Aug 29, 2019

@atomiks I just updated GitHub pages, old link should redirect to the new website anytime now (ETA: 10-15 minutes to work properly), so that's a big one dealt with. I already asked @fejes713 to check the JS and overflow issues so we can get on them tomorrow morning and I will think of a good solution for the scroll issue, too. If you have any ideas and can help us, I'd love the help. 😉

It's live already, old link redirects to new website now!

@fejes713
Copy link
Contributor

I am currently investigating overflowing issues but can't seem to find any. I know they exist because I found them a couple of days ago, but can't seem to find them now. Could anybody point me to the places of those problems?

@fejes713
Copy link
Contributor

The problem with JavaScript is here:

          {
            cardCodeJs &&
            <script>
              {`function()(${snippetData.code.js})();`}
            </script>
          }

This doesn't execute JavaScript code. This (function(){${snippetData.code.js} })(); would trigger the function if it was on page load since it's IIFE.

As far as I know, injecting script tags like this isn't possible probably due to security reasons.

Does anyone have an idea of how to approach this?

I even tried to execute code directly in the browser (Mouse cursor gradient tracking to be precise) and it didn't work either. document.querySelector would pick up the correct HTML nodes but the setProperty wouldn't work as expected. Instead, it would add

{
 0: '--x',
 1: '--y'
}

to the style object of the selected element.

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

Some overflow problems with elements in the snippets

Seems to only be the "Fullscreen" snippet

Some snippets don't gel properly with night mode (as it wasn't accounted for originally) and are hard to see

Maybe force the demo part to be white? That would look weird though.

+ Other things I've noticed

  • Remove scale hover animation for snippets since you can interact with them, add a clear focus ring when tapping a snippet instead?
  • Make it clear that they need to click a card to view the code
  • Allow scrolling on left and right side of the "scrolly" container (IMO use a fixed sidebar for the starter template instead), the UX seems kind of odd right now
  • Make the colored header span the full width of the screen

Btw I don't know the code yet so if someone familiar could make these changes instead that would be great 🙏

@Chalarangelo
Copy link
Owner

@fejes713 I think I have one idea on how to fix the JS snippet issue, let me take a stab at this and I will report back with results.

@atomiks

  • If overflow is an issue only on fullscreen, I will see if I can figure it out. It's a minor problem then and should be easy to fix via the snippets' code if necessary.
  • Scroll returning to its last state is something I have been thinking about, I have a proposal but the chances it works might be low.
  • The scaling hover animation is something I suspected could cause trouble on the CSS website, I was planning to review it today anyway, so that's definitely something I will deal with.
  • The whole clicking a card thing is explained at the top (which oddly enough reads click on the snippet title), but I will try to replace the animation with something better and more understandable on this website, as we definitely have to.
  • Scrolling does feel a tiny bit odd, I agree. I will look into it very soon.
  • Header width is something I will try, I think I know how to fix it in combination with the previous point.

@Chalarangelo
Copy link
Owner

Chalarangelo commented Aug 30, 2019

Update

  • The overflow issue in Fullscreen snippet is gone (just a box-sizing fix did the trick).
  • JS execution works already - although the gradient mouse tracking has some problems with setting --x and --y.
  • As far as the hovering of cards is concerned, I rolled back to clicking title to access snippet instead of the whole card (only this repo) and I added a button at the end of the card that reads View snippet, as the clicking the whole card functionality has an issue with clickable components in the demos.
  • The dark mode issues will be addressed manually, at least for now, and we should take care to check for them in future submissions.

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

Scroll returning to its last state is something I have been thinking about, I have a proposal but the chances it works might be low.

Maybe the scrolly container (instead of the body being scrolled) causes this? Otherwise the scroll position might need to be stored in state and read when navigating back

@Chalarangelo
Copy link
Owner

@atomiks This is a CSS grid issue at its core. I will use Redux to actually restore the scroll if I can. If you have any other ideas, feel free to pitch in.

Meanwhile, I pushed the manual fixes for dark mode on snippets, they should help it all stick together better.

I will be looking into the scrolling issue, container style, etc. in the next couple of hours, but I think I can fix it all today. Some of these fixes will probably be applied to all the other repos, too.

@atomiks and @fejes713 can either one of you figure out the gradient mouse tracking snippet issue? That one is bothering me and I do not have a lot of ideas...

@Chalarangelo
Copy link
Owner

I have just updated the css so that the container is actually set up properly and the scrollbar is moved to the far right of the screen so that it won't feel odd. I will push this same update to other repos. Unfortunately, the way it is now set up doesn't seem like the splash logo at the top can span the whole width, but I might have an idea to fix this via CSS.

The scroll retention when changing pages will be the last fix I want to push, as it is probably the most complex one.

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

I think the gradient one has to do with the offsetParent calculation, in the new UI it doesn't need it. (Although I thought it was meant to just "work" before in any situation but I guess not lol)

  var x = e.pageX - btn.offsetLeft // - btn.offsetParent.offsetLeft
  var y = e.pageY - btn.offsetTop // - btn.offsetParent.offsetTop

Edit: nvm I'm wrong, but it has to do with the x and y being wrong.

@Chalarangelo
Copy link
Owner

Chalarangelo commented Aug 30, 2019

@atomiks Ok, I'll test that. I figured as much that the calculation is somehow wrong, but I can't quite put my finger on it. It's a snippet-specific issue, not a website-wide problem, so we will figure this out on its own. JS does work on the website, though, so that's a lot of progress.

Logo placement and scroll are updated, scroll retention is the only thing missing. We are getting closer!

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

This should work:

var rect = e.target.getBoundingClientRect();
var x = e.clientX - rect.left;
var y = e.clientY - rect.top;

@Chalarangelo
Copy link
Owner

Ok, I'll test and update the snippet.

@Chalarangelo
Copy link
Owner

it works perfectly, @atomiks. Pushing right away.

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

The splash header is offset all the way to the right for some reason.

I think you need to remove the padding from .content, remove the left from .splash, and nes the snippets inside a new container with the padding

@Chalarangelo
Copy link
Owner

Chalarangelo commented Aug 30, 2019

That might mess up a lot of styles. What is the issue with the .splash exactly? It looks ok to me on Chrome, on dev mode. It's a Gatsby issue on the production mode, let me fix it real quick.

@Chalarangelo
Copy link
Owner

Chalarangelo commented Aug 30, 2019

@atomiks, wait for the next Netlify deployment to complete and test again in 5 minutes - it should work then.

Live and working as expected

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

Yup seems good @Chalarangelo

Some final CSS polishments:

  • Splash header separator doesn't work in night mode
  • Make dark mode text (and icon etc) color #e0e3f2
  • Make "View Snippet" background color #5d79ff and makes its font-size: 0.9rem and border-radius: 0.5rem
  • Make the snippet's padding 1rem 1rem 2rem so that the "View Snippet" doesn't look weird hanging off the end of the card (extra bottom padding)
  • Overflow scroll gradient snippet needs the ::after element to have a width of 250px
  • Remove "Snippet List
    Click on a snippet’s name to view its code or a tag name to view all snippets in that category." imo it's redundant
  • Make each category heading much larger with more spacing and center it as it's hard to see when scrolling down, it sort of blends together

There's probs more but I can do them later if need be, it should be mostly ready after that

@Chalarangelo
Copy link
Owner

  • Splash header separator doesn't work in night mode

Will push momentarily.

  • Make dark mode text (and icon etc) color #e0e3f2

Will also push momentarily.

  • Make "View Snippet" background color #5d79ff and makes its font-size: 0.9rem and border-radius: 0.5rem
  • Make the snippet's padding 1rem 1rem 2rem so that the "View Snippet" doesn't look weird hanging off the end of the card (extra bottom padding)

Will also push momentarily, I thought it looked cool hanging off the card, but no problem, we'll go with your option.

  • Overflow scroll gradient snippet needs the ::after element to have a width of 250px

Will fix as soon as I can, not sure what is not correct there and why.

  • Remove "Snippet List
    Click on a snippet’s name to view its code or a tag name to view all snippets in that category." imo it's redundant

This is here for two reasons: First, accessibility and second, consistency with other repos. Think about it and get back to me if you are absolutely certain this is redundant.

  • Make each category heading much larger with more spacing and center it as it's hard to see when scrolling down, it sort of blends together

Also, should be consistent with other repos, but we might actually make these larger org-wide to make them pop more.

TL;DR: Give me 20 minutes.

@Chalarangelo
Copy link
Owner

@atomiks Styles updated as promised. Made minor changes (i.e. 0.9rem is 0.875rem to get 14px instead of a sub-pixel value that looks weird in some browsers, changed the padding of the cards a little bit more to make the bottom area spacing more consistent with the rest of the card and created more of a palette from the dark mode text color you suggested as to show some difference between parts of the page), but other than that what you suggested.

At this point, I only need to figure out the scroll memory issue which proves to be a bit of a hassle. i will get back to you on this one, soon. Might require a little more trial and error and I might have to take a break, so I will probably get this done later today.

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

Will also push momentarily, I thought it looked cool hanging off the card, but no problem, we'll go with your option.

I actually did mean to leave it hanging off, just not as much as it was. I should've worded that better. Looks fine now anyway,

The splash shape separator still seems to not work, everything else seems good.

@Chalarangelo
Copy link
Owner

Chalarangelo commented Aug 30, 2019

@atomiks Forgot the separator, sorry, will do it right away! 😉 Pushed it already, waiting on the Netlify build

I also think it looks better if it doesn't hang off, shadows did look a bit weird actually, so better the way it is.

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

Definitely had a nice glow-up overall, thanks @Chalarangelo!

(I promise there's no more that I won't do later now):

  • It's SUPER slight but on day mode, the shape separator on the splash is slightly the wrong color
  • Make "CodePen" button the same styling as "View snippet"
  • Reduce the code block (.card .card-code) padding to a flat 1rem
  • Change the HTML/CSS/JavaScript gradient badge things to have color: white on night mode as well as it's a bit hard to read

At this point, I only need to figure out the scroll memory issue which proves to be a bit of a hassle. i will get back to you on this one, soon. Might require a little more trial and error and I might have to take a break, so I will probably get this done later today.

Since the content is in a scrollable container, you'd probably need to store the .content node's scrollTop value when clicking away and set it when returning back (or store it onScroll, if it's not a perf issue). Is that something you tried? Maybe apply the logic onRouteUpdate in the gatsby browser API file?

@atomiks
Copy link
Contributor Author

atomiks commented Aug 30, 2019

Also something weird I noticed, is that you need to click the "Night mode" icon directly, even though there's padding around it (which changes to the hand cursor icon), I thought it was buggy. The other icons don't require a "direct" click

@Chalarangelo
Copy link
Owner

Chalarangelo commented Aug 30, 2019

@atomiks

  • It's SUPER slight but on day mode, the shape separator on the splash is slightly the wrong color

Done

  • Make "CodePen" button the same styling as "View snippet"

Done

  • Reduce the code block (.card .card-code) padding to a flat 1rem

Done (the .card style is a tiny bit differentiated, but it looks like 1rem everywhere, bottom padding is just off due to positioning, nothing to worry about).

  • Change the HTML/CSS/JavaScript gradient badge things to have color: white on night mode as well as it's a bit hard to read

Done

Also something weird I noticed, is that you need to click the "Night mode" icon directly, even though there's padding around it (which changes to the hand cursor icon), I thought it was buggy. The other icons don't require a "direct" click

I know it's odd, I have it in mind, it's not a dealbreaker but it's something I want to come back and polish up as soon as I can.

Since the content is in a scrollable container, you'd probably need to store the .content node's scrollTop value when clicking away and set it when returning back (or store it onScroll, if it's not a perf issue). Is that something you tried? Maybe apply the logic onRouteUpdate in the gatsby browser API file?

I haven't tried this ever, but it sounds plausible. I was pretty much trying to accomplish this by coding it myself, but I'll try your idea first.
Looking at this right now, looks very promising!

Wait for the next Netlify build to see changes

@Chalarangelo
Copy link
Owner

@atomiks I couldn't be more grateful for the suggestion of using the onRouteUpdate, which in combination with onPreRouteUpdate and some simple logic can act as a scroll memory logic storage for the entire website (plus the rest of the projects). I am pushing this as we speak and it should work on the next build. Along with a few other changes, I will push this to all repo websites and we should be ready to launch very soon.

@Chalarangelo
Copy link
Owner

Just added the final touches on my last commit, I am closing this as I think I have resolved every single bullet point that was raised here. Thank you so much for all the constructive criticism and feedback on the implementation, it really helps make our work shine!

@lock
Copy link

lock bot commented Dec 18, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants