Skip to content

Reading view: action icons position#6297

Merged
Alkarex merged 24 commits intoFreshRSS:edgefrom
math-GH:reading-view-action-icons
Jun 13, 2024
Merged

Reading view: action icons position#6297
Alkarex merged 24 commits intoFreshRSS:edgefrom
math-GH:reading-view-action-icons

Conversation

@math-GH
Copy link
Contributor

@math-GH math-GH commented Apr 13, 2024

inspired by #6261 from @joshka

(default): above title

grafik

grafik

in authors and date row

grafik

grafik

top line article icons configs respected

grafik

How to test the feature manually:

  1. go to config reading, section Articles: header/footer
  2. edit the option
  3. see in reading view

Pull request checklist:

  • clear commit messages
  • code manually tested

@math-GH math-GH added the UI 🎨 User Interfaces label Apr 13, 2024
@math-GH math-GH added this to the 1.25.0 milestone Apr 13, 2024
@math-GH
Copy link
Contributor Author

math-GH commented Apr 13, 2024

@joshka Please have a look here :) I added you into the CREDITS.md to mention, that parts of this code was made by you (in #6261).

@joshka
Copy link

joshka commented Apr 15, 2024

LGTM. Thanks for the change!

@Alkarex
Copy link
Member

Alkarex commented Apr 25, 2024

Tests failing

math-GH and others added 4 commits June 11, 2024 19:56
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
math-GH and others added 3 commits June 12, 2024 23:03
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
@Alkarex Alkarex merged commit 20f1331 into FreshRSS:edge Jun 13, 2024
@math-GH math-GH deleted the reading-view-action-icons branch June 13, 2024 19:59
@Alkarex Alkarex modified the milestones: 1.25.0, 1.24.2 Jul 12, 2024
<label class="group-name" for="show_article_icons"><?= _t('conf.reading.article.icons') ?></label>
<div class="group-controls">
<select name="show_article_icons" id="show_article_icons" data-leave-validation="<?= FreshRSS_Context::userConf()->show_article_icons ?>">
<option value="t" <?= FreshRSS_Context::userConf()->show_article_icons === 't' ? ' selected="selected"' : '' ?> data-input-visible="true"><?= _t('conf.reading.article.icons.above_title') ?></option>
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@math-GH math-GH Jul 22, 2024

Choose a reason for hiding this comment

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

should it not caught by the pipeline?

I use VS Code that respects the .editorconfig. But there is no annotation at this place.

Something is here broken I guess

Copy link
Member

@Frenzie Frenzie Jul 22, 2024

Choose a reason for hiding this comment

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

It's listed right at the start:

end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true

Edit: Maybe it only works if you open the root folder, but not if you open a subfolder or a single document? Although VS Code trims trailing whitespace by default as well.

Copy link
Member

Choose a reason for hiding this comment

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

In VS Code, make sure you have the corresponding extension https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig
We do not have a CI test for that at the moment (I plan to make an update for PHPCS soon, which might catch it)

Copy link
Member

Choose a reason for hiding this comment

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

As promised #6666
This will now catch such cases as part of the CI, also also offer automatic fixes with

make test-all
# or isolated:
composer run-script phpcbf

@math-GH math-GH mentioned this pull request Jul 23, 2024
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.

4 participants