Skip to content

Conversation

@kolcz
Copy link
Contributor

@kolcz kolcz commented Jul 12, 2025

Description

This PR addresses issue #827. In this PR I made font, in SearchResultsDock to match application settings parameters during initialization and when font is changed in PreferencesDialog.

Changes Made

  • Remove setting different font size on MacOS,
  • Add setFont() method to SearchResultsDock,
  • Added running setFont after SearchResultsDock initialization,
  • Connect ApplicationSettings::fontNameChanged and ApplicationSettings::fontSizeChanged to setFont method.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change
  • Documentation update

Testing - on Windows 11 and Debian 12 bookworm

  • Font in SearchResultsDock, Editor are the same during initialization and identical to font specified in PreferencesDialog,
  • After changing font size by clicking arrows, both fonts in Editor and SearchResultsDock are modified,
  • After changing font size by typing new value, both fonts in Editor and SearchResultsDock are modified,
  • After changing font family by selecting from the list, both fonts in Editor and SearchResultsDock are changed,
  • After changing font family by typing new value, both fonts in Editor and SearchResultsDock are changed.

Additional Note

Someone need to test it on MacOS.

@kolcz kolcz marked this pull request as draft July 12, 2025 18:00
@dail8859
Copy link
Owner

Thanks for this!

I think this is a good first step. It would be best to also go ahead and always set the font of the tree view to the default font and size that is specified in the ApplicationSettings. This can be changed in the future but it's a logical step so that things will be consistent.

@kolcz
Copy link
Contributor Author

kolcz commented Jul 13, 2025

You're welcome. :)
Yeah, sure. Good idea! I was a little surprised when I found out that Debian by default has font "Liberation Mono" with size 12, so it will be a good idea to unify that.

@kolcz kolcz marked this pull request as ready for review July 18, 2025 21:35
@kolcz kolcz marked this pull request as draft July 18, 2025 21:40
@kolcz kolcz marked this pull request as ready for review July 18, 2025 21:40
@kolcz
Copy link
Contributor Author

kolcz commented Jul 18, 2025

@dail8859 Could you take a look at changes? I can't request a review by github now. :/

@dail8859
Copy link
Owner

dail8859 commented Aug 1, 2025

Apologies for the long delay.

I consolidated the changes into just the SearchResultsDock. It also got rid of overloading the setFont() method already defined by QWidget. This way the MainWindow doesn't need do anything else specifically for the dock. It might be a bit terse for now but it seems to work correctly.

Let me know if you have any thoughts.

Copy link
Contributor Author

@kolcz kolcz left a comment

Choose a reason for hiding this comment

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

I think it's better to implement all logic inside SearchResultsDock. It will isolate particular docks windows and when we implement a lambda function like we don't pass variables from another objects. My problem was that I didn't know how to get ApplicationSettings from inside SearchResultsDock class.
I wonder what does resizeColumnToContents do? As I tested when font increase the text can move out of window space. So I don't see how this function work.
I tested both on Windows 11 and Debian and it works as it work as intended. I think we can merge Pull Request. Or you see another way to improve it?

@dail8859
Copy link
Owner

dail8859 commented Aug 4, 2025

Technically you can instantiate a ApplicationSettings anywhere you like. The instantiation is actually quite light weight since it is based on QSettings. However, to properly trigger and also receive notifications you need to make sure to get to the pointer to the one that is owned by the NotepadNextApplication instance.

I wonder what does resizeColumnToContents do?

Without this line, the column used for the line numbers does not dynamically resize leading to the line numbers getting :
resize

@dail8859 dail8859 merged commit 883c7ed into dail8859:master Aug 4, 2025
12 checks passed
@kolcz
Copy link
Contributor Author

kolcz commented Aug 5, 2025

Thank you for the explanation and help with new feature. Now I better understand the application.

@kolcz kolcz deleted the unify_font_size_in_search_result_dock branch August 9, 2025 17:00
matthewyang204 pushed a commit to matthewyang204/NotepadNext that referenced this pull request Dec 14, 2025
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.

2 participants