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

Mapco text display improvements #4711

Merged
merged 18 commits into from
Oct 27, 2022
Merged

Mapco text display improvements #4711

merged 18 commits into from
Oct 27, 2022

Conversation

patjennings
Copy link
Contributor

Closes #4695

@patjennings
Copy link
Contributor Author

ouch, automated testing is demanding, i got errors on minor things (from my point of view). should i fix everything for this PR to be accepted ?

@Alkarex Alkarex added this to the 1.20.1 milestone Oct 11, 2022
@Alkarex Alkarex added the UI 🎨 User Interfaces label Oct 11, 2022
@Alkarex
Copy link
Member

Alkarex commented Oct 11, 2022

The easiest is to run make fix-all (which I have just done) or the specific automated methods.

@Alkarex Alkarex requested a review from math-GH October 11, 2022 10:37
p/themes/Mapco/mapco.scss Outdated Show resolved Hide resolved
@Frenzie
Copy link
Member

Frenzie commented Oct 11, 2022

i got errors on minor things (from my point of view). should i fix everything for this PR to be accepted ?

Yes, it whines about it so I don't have to whine about it. ;-P

@Alkarex Alkarex added Theme 🖌 and removed UI 🎨 User Interfaces labels Oct 11, 2022
@Alkarex Alkarex changed the title Text display improvements Mapco text display improvements Oct 11, 2022
@patjennings
Copy link
Contributor Author

i'm struggling with the make fix-all command. My poor skills barely give me a clue on how to solve it. I let as is, sorry for that.

@Alkarex Alkarex requested a review from math-GH October 12, 2022 20:58
Comment on lines 1376 to 1379

.icon {
height: inherit;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you forgot to commit mapco.scss, that inserts this lines into the CSS file.

I would suggest to do another PR for changing the width/height behavior of .icon and its height. Let's do not do it with this PR. I can support you with a PR.

height: 1rem;
height: inherit;
Copy link
Contributor

Choose a reason for hiding this comment

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

see my comment above

phpcs.xml Outdated Show resolved Hide resolved
@math-GH
Copy link
Contributor

math-GH commented Oct 27, 2022

It is fine for me how it is now

@Alkarex Alkarex merged commit fc93776 into FreshRSS:edge Oct 27, 2022
@math-GH math-GH mentioned this pull request Oct 30, 2022
2 tasks
math-GH added a commit to math-GH/FreshRSS that referenced this pull request Nov 15, 2022
* Main font color from blue to regular dark

* newline added at the end of rtl css file

* Text display improvements (code, quotes) + minor UI fixes

* replaced 2 spaces with 1 tab bcz phpcs is whining about that

* make fix-all

* Mapco/base-theme - CSS cleaning

* make fix-all

* PR FreshRSS#4711 cleaning

* Exclude generated CSS from lint check

* Revert exclusion of generated CSS files

Co-authored-by: patjennings <[email protected]>
Co-authored-by: Alexandre Alapetite <[email protected]>
Co-authored-by: patjennings <[email protected]>
Co-authored-by: maTh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Can't distinguish links on mapco theme
4 participants