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

Fix Swage theme issues #2088

Merged
merged 2 commits into from Oct 30, 2018
Merged

Fix Swage theme issues #2088

merged 2 commits into from Oct 30, 2018

Conversation

pattems
Copy link
Contributor

@pattems pattems commented Oct 29, 2018

I fixed the issue with the active tab not being highlighted in the config view.

In order to make the configure-feeds buttons be able to adapt to fill the width of the aside, I had to add ID tags to them, as without this change, I couldn't target the buttons individually, and the only solution would be to make each button 50% the width of the aside, a less than ideal solution that would cut text in pretty much any language (except possibly languages which use logograms instead of alphabets).

Ideally, I'd like to add ID's to all of the UI components, which would greatly increase the themeability of FreshRSS without breaking any existing themes, but that's not work I'm going to take on without any sort of discussion first.

Fix highlighting issue in Config pages, prepare for variable-width config buttons
Add ID's to be able to individually target buttons inside the configure feeds (Subscriptions Management & Import/Export) buttons.
@Alkarex Alkarex added this to the 1.13.0 milestone Oct 30, 2018
@Alkarex Alkarex added the UI 🎨 User Interfaces label Oct 30, 2018
@Alkarex Alkarex merged commit 43e0b02 into FreshRSS:dev Oct 30, 2018
@Alkarex
Copy link
Member

Alkarex commented Oct 30, 2018

Thanks @pattems 😃
A couple of versions ago, there was this style fix #1980 to allow scrolling enough to the bottom to mark all articles as read. Could you please try to get the same in Swage?

@pattems
Copy link
Contributor Author

pattems commented Oct 30, 2018

@Alkarex sure, that's simple enough, just have to pull out a single line of CSS from the theme. I don't use scroll to mark as read, so I didn't realize that that was affecting a feature. Want a PR specifically for that, or should I wait until 12.1 is closer and roll it in as part of a PR fixing other issues that might pop up with the theme? I don't want to clutter up the queue with a bunch of single or couple line PR's if it's going to annoy anyone.

@Alkarex
Copy link
Member

Alkarex commented Oct 30, 2018

Yes please, another PR is fine. Too many PRs are not a problem (yet) :-)

@pattems
Copy link
Contributor Author

pattems commented Oct 30, 2018

I'll get it to you a bit later on, then!

Alkarex added a commit that referenced this pull request Nov 11, 2018
@Alkarex Alkarex mentioned this pull request Dec 16, 2018
javerous pushed a commit to javerous/FreshRSS that referenced this pull request Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI 🎨 User Interfaces
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants