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.

Sign up for GitHub

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

Jump to bottom

DYN-5426 Setting different font size when the height of the library is smaller #13578

Merged
merged 2 commits into from
Dec 1, 2022
Merged

DYN-5426 Setting different font size when the height of the library is smaller #13578

merged 2 commits into from
Dec 1, 2022

Conversation

filipeotero
Copy link
Contributor

Purpose

Sets the font size depending on the height of the library

Scale at 350% and resolution 3840 x 2160
image (3)

Scale at 150% and resolution 1920 x 1080
image

Scale at 100% and resolution 1920 x 1080
image

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

@QilongTang

FYIs

@RobertGlobant20

@QilongTang
Copy link
Contributor

Hi @filipeotero Is it because height is a direct reflection of the screen resolution?

@mjkkirschner
Copy link
Member

@filipeotero @QilongTang .... but what if I change the size of the dynamo window?

@QilongTang
Copy link
Contributor

@filipeotero @QilongTang .... but what if I change the size of the dynamo window?

My understanding is that the size of the library does not change, when Dynamo window size changed, user will have to scroll more or less depending on the new window size. @filipeotero What do you think?

@filipeotero
Copy link
Contributor Author

filipeotero commented Nov 29, 2022 •
edited
Loading

@filipeotero @QilongTang .... but what if I change the size of the dynamo window?

It changes the size of the font too

ezgif-2-645fe244c0

@QilongTang
Copy link
Contributor

QilongTang commented Nov 29, 2022 •
edited
Loading

@filipeotero @QilongTang .... but what if I change the size of the dynamo window?

It changes the size of the font too

Then I guess I am wrong then.. And this is not desirable..

@mjkkirschner
Copy link
Member

mjkkirschner commented Nov 29, 2022 •
edited
Loading

@Jingyi-Wen @hwahlstrom what do you think?

@Jingyi-Wen
Copy link

@Jingyi-Wen @hwahlstrom what do you think?

Hmm...Changing the library font size like this seems very confusing to me since the intention of making the window would most likely be wanting to see more content instead of making the font bigger

@filipeotero
Copy link
Contributor Author

If I set the font size through the dpi resolution, the font is not affected since through CSS it gets the browser's dpi, not the system's.
The dpi only changes if I zoom in to the Library.
A solution that I was thinking of is that we can grab the screen height of the user from Dynamo (using SystemParameters.PrimaryScreenHeight) and calculate the font size based on that.
Let me know what you guys think. @QilongTang @mjkkirschner

@mjkkirschner
Copy link
Member

@filipeotero you could also pass the DPI to the javascript side from c# and then use that to do your calculations.

@filipeotero
Copy link
Contributor Author

@QilongTang @mjkkirschner I'm calculating now the font size in the SetLibraryFontSize method.
Here are some examples of how it's working

1920x1080:
scaleCalculation
librarySizeChang

3840x2120 at 350%
Sem título

@QilongTang
Copy link
Contributor

The reported failure should be sporadic

@QilongTang QilongTang merged commit 2717d5b into DynamoDS:master Dec 1, 2022
QilongTang pushed a commit that referenced this pull request Dec 1, 2022
@filipeotero @QilongTang
DYN-5426 Setting different font size when the height of the library i…
228930e 
…s smaller (#13578)

* Setting different font size when the height is smaller

* calculate the font size based on the screen height
@QilongTang QilongTang mentioned this pull request Dec 1, 2022
Merged
8 tasks
QilongTang added a commit that referenced this pull request Dec 1, 2022
@QilongTang @reddyashish
@filipeotero @RobertGlobant20
Cherry-picks (#13587)
441b0d1 
* Fix CustomNode view layout after it is saved. (#13585)

* Fix CustomNode views after it is saved.

* Add test

* Update WorkspaceSaving.cs

* Single quote to double quote in js call functions (#13586)

* DYN-5286-MLNodeAutocomplete Adding Analytics  (#13581)

* DYN-5286-MLNodeAutocomplete-Analytics

I added the code for TrackEvent in 4 places:
- NodeAutoCompleteSearchControl.xaml.cs - when clicking the Low Confidence combobox for displaying the list of nodes.
- NodeAutoCompleteSearchViewModel.cs - when the Node Autocomplete is raised using the Recommented nodes option or the Object Types option.
- PreferencesView.xaml.cs when the Slider Confidence Level is updated.
And finally I've added additional Actions in IAnalyticsClient.cs

* DYN-5286-MLNodeAutocomplete-Analytics CodeReview1

Removed the Actions added in IAnalyticsClient.cs so we can re-use the existing ones (so the method calls were also updated).
Added Analytics track events in the Preferences panel when the NodeAutocomplete is se to ML Recomentations or Object Type.
The ExportSettings option and ImportSettings option were renamed to Export and Import (so the method calls were also updated).

* Update

* update

* Update

Co-authored-by: Aaron (Qilong) <[email protected]>

* DYN-5426 Setting different font size when the height of the library is smaller (#13578)

* Setting different font size when the height is smaller

* calculate the font size based on the screen height

Co-authored-by: reddyashish <[email protected]>
Co-authored-by: filipeotero <[email protected]>
Co-authored-by: Roberto T <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QilongTang QilongTang QilongTang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@filipeotero @QilongTang @mjkkirschner @Jingyi-Wen