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

PR: Prep for V2 release with over a year of improvements and additional features for Spyder site #8

Merged
merged 44 commits into from Sep 16, 2020

Conversation

CAM-Gerlach
Copy link
Member

There has been well over a year of downstream enhancements and features on the theme after using it on a variety of sites, including a school club, a rocket launch tour organization, and even an interactive scientific data dashboard. This PR pulls those back in to the upstream theme, along with a number of further improvements and changes needed to finally port Spyder's own website from Lektor-Icon V0 to this beta of V2 as part of spyder-ide/website-spyder#177 . It should be fully backwards-compatible with only minor styling changes (mostly straight improvements); the example site built with no errors and a similar or improved appearance with no changes after V2.

This also adds Netlify support, so that the theme's changes can be previewed on the example site.

A followup PR will do the final tasks necessary for he the formal V2 release (not included here to avoid blocking spyder-ide/website-spyder#177 ): updating the test site to take full advantage of the new lektorproject and theme options, updating the readme with the new theme parameters and other changes, doing final conformance/style checks, and updating the changelog, readme, contributing guide, roadmap, and version file for the release.

CAM-Gerlach and others added 30 commits May 28, 2019 20:50
Extend Content-Security-Policy for launch countdown
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks a lot @CAM-Gerlach for your work!

I have two minor comments about it:

  • Section headers are now underlined in red

    Selección_088

    whereas in the current version we use a light gray color for that

    Selección_089

    I'd prefer to leave the current style for now (we can discuss to change it in another PR).

  • The red of "Open chat" is not the same as the one used in the rest of the page. It's a lighter shade, as can be seen here:

Selección_090

@CAM-Gerlach
Copy link
Member Author

Sorry for the confusion, this PR is just the V2 changes to the underlying theme, rather than those to the Spyder site itself, which uses different flowblocks, theme settings and custom styles along with incorporating the improvements here (and in V1, merged long ago). So its likely easiest for you to just review it there so you can see how it looks on the actual Spyder site, and if I do have to fix something on the theme level (like the chat icon position, in the future) I'll make it here, like we did reviewing @dalthviz 's sphinx theme.

Section headers are now underlined in red

Ah yes, this was done years ago in v1.0 and is used by the many other sites that now use Lektor-Icon, so the simplest way to change it without breaking backward compat is just do it in one line in the custom stylesheet on the website repo. If you guys agree on that I'll go ahead with it, it will only take a few seconds; just let me know.

The red of "Open chat" is not the same as the one used in the rest of the page. It's a lighter shade, as can be seen here:

Actually, this is the current intended behavior (though in V1 I believe it was reversed); the buttons all use theme_pipe_color (a lighter variation of the accent color, so named because it was originally used just for the pipes in the header and footer, so they'd have higher contrast from the background, and the name was kept for backward compatibility) as their color when not hovered, and the darker theme_accent_color (the red color used most other when the user hovers over or presses them (as is typical for buttons).

We could add a new theme option, theme_dark_accent_color or similar, to use for this purpose (which we'd set to the darker red Isabella came up with, at least for our site) and shift the normal button color down to theme_accent_color. Whatever you all like best.

@ccordoba12
Copy link
Member

Section headers are now underlined in red

We discussed this with Isabella and agreed they look nice. So there's no problem with that at the end.

We could add a new theme option, theme_dark_accent_color or similar, to use for this purpose

Ok, sounds good to me.

@ccordoba12
Copy link
Member

I think this is almost ready! Thanks @CAM-Gerlach!

My last comments:

  • The logo font size is a bit bigger here (2.2em) than in our current website (2em):

    Selección_098

  • The footer background color is slightly different (#333333) than in our current website (#414042):

    Selección_099

  • We agreed on having four lines in the footer:

    • Social media
    • Copyright and licenses
    • Where template and icons were taken from.
    • Contributors.

    Since there are five lines now, you could remove Maintained by Spyder IDE in the last one and add Rewirte by C. A. M Gerlach to the fourth one (after Lektor port by Dalthviz).

@CAM-Gerlach
Copy link
Member Author

@ccordoba12 I apologize for any confusion; as I mentioned previously, this PR is just the underlying theme improvements that will affect all sites using the theme (quite a few now, at least one with >500 Github stars), while how the changes actually look on Spyder's own site is the subject of spyder-ide/website-spyder#177 . I have addressed each of these issues there; your first and second items are already resolved.

As for the third, as I discuss there the theme-side credit block cannot be changed site-side (for both technical and legal reasons) and will affect all our users. The above proposal (left) presents several problems: even with cut-down text and hand-tuned line breaks, its still two lines on smaller screens/mobile (where space matters the most), has a less clean and balanced layout vs. the current (right) and doesn't include the link to the Spyder site, which gives us credit, increases our visibility and boosts our search rank, making it easier for users to find our site and docs.

image

However, after trying various techniques, the one workable approach I found to get the theme credit block down to 3 lines was reverting to a cut-down version of the Lektor-Icon v1 text:

image

So, while I prefer the current implementation, if 6 vs. 5 total lines in Spyder site footer (7 vs. 6 on mobile) is a deal-breaker for you, I can go ahead and implement this instead. Just lmk.

@ccordoba12
Copy link
Member

I have addressed each of these issues there; your first and second items are already resolved.

Ok, thanks for that! And sorry for the confusion about where changes need to go.

The above proposal (left) presents several problems: even with cut-down text and hand-tuned line breaks, its still two lines on smaller screens/mobile (where space matters the most), has a less clean and balanced layout

Ok, I haven't considered issues in mobile screens. That's a very good point, thanks for bringing it up!

the current (right) and doesn't include the link to the Spyder site, which gives us credit, increases our visibility and boosts our search rank, making it easier for users to find our site and docs.

Good points too.

So, while I prefer the current implementation, if 6 vs. 5 total lines in Spyder site footer (7 vs. 6 on mobile) is a deal-breaker for you, I can go ahead and implement this instead. Just lmk.

Given your rationale, I think we could go with 5 lines. But let's see what @dalthviz thinks about this.

@dalthviz
Copy link
Member

Sure go ahead with 5 lines 👍

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Sep 16, 2020

So @ccordoba12 are you saying I should go ahead with this one

image

or keep the new one

image

Not sure which 5 lines you're referring to jeje. Whatever you want I'll go ahead with but I want to make sure I know which that is :).

@ccordoba12
Copy link
Member

or keep the new one
image

Sorry for the confusion. I think we should keep the new footer.

@dalthviz?

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @CAM-Gerlach for your hard work on this one! The enhancements you added to the theme look great!

@ccordoba12 ccordoba12 merged commit 47d52c5 into spyder-ide:master Sep 16, 2020
@CAM-Gerlach
Copy link
Member Author

@ccordoba12 Thanks, Merging this does commit us to merging spyder-ide/website-spyder#177 without further theme changes (unless we want to open another PR and relink it to that) but at least from my side, it should be ready; I addressed all the deltas except the one that wasn't technically practical (at least without major work by someone smarter than me, like @dalthviz ).

In any case, as soon as that gets merged, I can proceed immediately with the followup PR, which will remove Spyder Reports, make it use the same logo and style as the docs navbar, reduce the size of the screenshot to prevent overflow, link the panes to their docs and the sponsors to their sites, and fix some textual issues and out of date content on the main page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants