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

Adding text formatting for hits #9686

Merged
merged 6 commits into from
Mar 22, 2023

Conversation

vigneshdurairaj
Copy link
Contributor

@vigneshdurairaj vigneshdurairaj commented Mar 19, 2023

Compulsory checks

Preview Give feedback

Fixes #8797

As first-time contributors, we would like to ensure that our submission adheres to the community guidelines. Consequently, we have submitted this Pull Request as part of our university project. Our team member, @sambhav97, has been assigned to the issue.

We believe that displaying numbers such as 1000 as 1k is a design choice. To provide a visual representation, we have included a screenshot displaying how the entries appear with this change. If there are any oversights or areas for improvement, please do not hesitate to inform us.

When the entries are 1000
Screenshot 2023-03-19 at 10 21 43 AM
When the entries are 1500
Screenshot 2023-03-19 at 10 22 03 AM
When the entries are 100,000
Screenshot 2023-03-19 at 10 22 35 AM

@Siedlerchr
Copy link
Member

I like the idea, just a question for the last screenshot? Does that mean 100,000 entries? or just 1000?
I would assume 100k= 100,000

@calixtus
Copy link
Member

looks just like a typo to me, since he had 1000 entries = 1k in his first screenshot...

@calixtus
Copy link
Member

grafik

@vigneshdurairaj
Copy link
Contributor Author

I like the idea, just a question for the last screenshot? Does that mean 100,000 entries? or just 1000?
I would assume 100k= 100,000

Yes it is 100,000. Changed the description now accordingly.

@Siedlerchr
Copy link
Member

I tested it, it's a nice idea. However, it does not resolve the original issue:
Set font size to 14 in Appearance.

There is still something not right with the padding regarding the font size. You might need to check with https://github.com/JonathanGiles/scenic-view to see what's going on.

grafik

@calixtus
Copy link
Member

About the size of the controls, labels and stuff: Controls should be sized in relation to the base font size of the main application window. See the documentation on jfx css measure units: docs.oracle.com/javafx/2/api/javafx/scene/doc-files/cssref.html#typelength

@vigneshdurairaj
Copy link
Contributor Author

vigneshdurairaj commented Mar 21, 2023

We have analyzed and improved the padding and font scaling in the UI.
Our changes include:
Examining the CSS padding, which accommodates the arrow and icon elements.
Identifying UI breakpoints by applying text formatting to observe where the UI breaks.
Adjusting font scaling by changing the font size units from percentage to em.
Implementing a logic in Java that reduces the font size by 0.20 em for every 4 steps after the font size reaches 17.
These changes improve the UI's responsiveness and ensure a consistent appearance across different font sizes.

With font size default
default
With font size 14
size_14
With font size 18
size_17
With font size 22
size_21
With font size > 26
size_22

@Siedlerchr
Copy link
Member

Yeah! Really great work, tested it, works fine! Thanks for digging into this :)
Codewise looks also good to me.
Please add a changelog entry, then you can set it to ready for review.

@vigneshdurairaj vigneshdurairaj marked this pull request as ready for review March 22, 2023 07:03
Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

😍
Took the liberty tho shorten the changelog a bit.

@calixtus calixtus merged commit 9d9652c into JabRef:main Mar 22, 2023
@Siedlerchr
Copy link
Member

@vigneshdurairaj Unfortunately, there was an issue with the update of the item count. However, I was able to adapt your code a bit. FYI: #9709

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.

Groups only show 4 digit number of entries
4 participants