Skip to content

Conversation

@Ammar64
Copy link
Contributor

@Ammar64 Ammar64 commented Aug 22, 2025

I fixed UI in RTL Mode.
I think the calculator layout should stay as it is in RTL mode.
Those parenthesis were annoying.

Before After
image image

I also improved the performance of the Dropdown menu of the UnitConverter screen.
The difference is significant in currencies screen as it has the largest dropdown menu.
It now opens much faster since I'm using LazyColumn.

@Ammar64
Copy link
Contributor Author

Ammar64 commented Aug 24, 2025

I think we should move the calculator history to a separate screen to save some space on the screen.
and that calculator history box doesn't scroll down to the latest operation when you tap =

Copy link
Member

Choose a reason for hiding this comment

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

Please remove the commited changes to .idea.

Copy link
Contributor Author

@Ammar64 Ammar64 Aug 28, 2025

Choose a reason for hiding this comment

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

Oops.
I think it should be added to .gitignore.
My IDE automatically edited these files without me noticing.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I agree.

Comment on lines 149 to 151
val itemHeight by remember {
mutableStateOf(45.dp)
}
Copy link
Member

Choose a reason for hiding this comment

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

itemHeight doesn't need to be a mutable state since it's never modified, you can place use 45.dp directly in dropDownHeight.

}
LazyColumn(
modifier = Modifier
.width(width)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.width(width)
.fillMaxWidth()

didn't work here?

Copy link
Contributor Author

@Ammar64 Ammar64 Aug 28, 2025

Choose a reason for hiding this comment

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

I tried .fillMaxWidth() and it crashes whenever you open the dropdown menu

Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

I've also noticed the performance issues when opening the currency converter dropdown, so these changes are very welcome :)

@Bnyro
Copy link
Member

Bnyro commented Aug 27, 2025

I think we should move the calculator history to a separate screen to save some space on the screen.

I'm not sure about that, I find it quite useful to have the history on the same screen. Sometimes I need some result of a previous calculation for the next expression I want to calculate, so it makes sense to see it at the same glance.

and that calculator history box doesn't scroll down to the latest operation when you tap =

True, we should fix that.

@Bnyro
Copy link
Member

Bnyro commented Aug 27, 2025

and that calculator history box doesn't scroll down to the latest operation when you tap =
True, we should fix that.

Fixed with ddbb5ce.

@Ammar64
Copy link
Contributor Author

Ammar64 commented Aug 28, 2025

I'm not sure about that, I find it quite useful to have the history on the same screen. Sometimes I need some result of a previous calculation for the next expression I want to calculate, so it makes sense to see it at the same glance.

It is hard to find the result of previous calculation because you only see 2 rows in the history box and you have to keep scrolling until you find it.
Same thing with the currency converter screen.
It's hard to find the currency you want in the dropdown menu and it's also hard to find the currency you want to convert to in the results list.

@Ammar64
Copy link
Contributor Author

Ammar64 commented Aug 28, 2025

It's hard to find the currency you want in the dropdown menu and it's also hard to find the currency you want to convert to in the results list.

Adding search in the currency screen may solve the issue?

@Bnyro
Copy link
Member

Bnyro commented Aug 28, 2025

It is hard to find the result of previous calculation because you only see 2 rows in the history box and you have to keep scrolling until you find it.

At least from my experience it's simple enough because I don't do more than 10 calculations in a session, I guess we won't find an agreement here :)

It's hard to find the currency you want in the dropdown menu and it's also hard to find the currency you want to convert to in the results list.

I agree on that, pull requests welcome.

Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

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

Thank you!

@Bnyro Bnyro merged commit 17c7ef8 into you-apps:main Aug 28, 2025
1 check failed
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