-
-
Notifications
You must be signed in to change notification settings - Fork 859
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
Swage theme cleanup #4493
Swage theme cleanup #4493
Conversation
Have you tried |
Ready for review. To fix/improve this theme was much more time consuming as I planned before. |
Ping @pattems if you are around. Any comment welcome :-) |
This looks like a great work, well done @math-GH 👍🏻 |
const nav_menu = document.querySelector('.nav_menu'); | ||
let nav_menu_height = 0; | ||
|
||
if (getComputedStyle(nav_menu).position === 'fixed' || getComputedStyle(nav_menu).position === 'sticky') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not tested yet. Any impact on the other themes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it with Origine. So it should not have an negative impact on other themes.
Let me explain this lines:
The changes in the JS file are needed when the user navigates with the left, top, right arrow navigation through the feed list. An additional factor was needed to calculate the correct position of the opened article.
Ups, a few conflicts now |
Thanks for the ping! I really like 99% of these changes; they definitely round out the rough edges. The one thing I will say I really dislike is moving the new article banner; being able to stick it in the sidebar where it made more sense to me was one of the main reasons I made this theme in the first place. What I'd prefer to see happen would be using @media rules to dump the new article banner out of the sidebar and into the stream only if the sidebar's collapsed, but I can always tweak my local copy of the theme if that doesn't sound reasonable. I'd offer to make that change myself but I'm drowning in other projects at the minute and not in a position to set up a dev environment to do proper testing. |
Thanks @pattems for your quick feedback and the insight that it is very important for you to have the banner on that place. I reverted it. Now the banner is back on the place. |
Let's merge and come back to it in another PR if @pattems has more comments |
@Alwaysin I cannot reproduce it. I can scroll. When you move with your mouse cursor over the dropdown menu a white scroll bar will appear Which browser do you use? |
ah ok. now I got it. It does not happen in the mobile view, it happens in the "desktop view". I checked the demo and it happens there too, so this issue is not new with this PR. To be honest: If you have this small screen (bigger than mobile view, smaller than a normal screen) than we could have issues also on other places. Please let me know your standard resolution. |
thanks for this information. I can confirm that 150% is the recommended zoom in Windows10 for a 14" notebook with this resolution. |
Login button, login screen, register form
before
After
much more beautiful. The login button is styled. Every page has the blue header banner. The form is clear styled now.
Form:
<legend>
<input>
<select>
before
very grey. Grey inputs and selects looks like disabled elements.
select has not the same height like the input
after
highlighted
<legend>
, fresh inputs and selects with a grey border and same height.(I personally would not highlight the
<legend>
but I wanted to keep the idea of the theme authorMobile view: header
before
not fixed header. The width of the search input was not good. The user query button was on the left side, above the read button. The "mark as read" button was not visible
after
The header is sticky to the top of the page. The content scrolls below
Dropdown menus
before
after
The config icon is now on the right side. In the user query menu the config icon is now placed by
position: absolute
insteadfloat: right
. When hovering over the icon, the background gives feedback with a lighter blue background color.Box layout (feed/category management, idle feeds)
before
Grey text on grey background. The config button of the feeds is weird (it is a white cog icon on grey background).
White category names of dark background. Dark feed config icon. The empty category "Add category" has now a better visible grey box border. Hover over the cog icons changes the color of the cog to orange.
Global view
before
the header of the overlay is behind the menu banner. And the width is wider than necessary.
after
aside navigation bar
before
after
a little bit is now a structure visible
About FreshRSS link (anonym. mode)
before
after
Changes proposed in this pull request:
How to test the feature manually:
Pull request checklist: