appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
32.86k stars 3.58k forks source link

[Bug]: Changing font causes visual distortion; more noticeable than what used to happen on the base DP #33331

Open ramsaptami opened 3 months ago

ramsaptami commented 3 months ago

Is there an existing issue for this?

Description

changing font causes visual distortion; more noticeable than what used to happen on the base DP

Steps To Reproduce

  1. Change app theme one by one as seen in the video and notice the visual "jiggle" widgets do to adjust spacing

Public Sample App

No response

Environment

Deploy Preview

Severity

High

Issue video log

https://www.loom.com/share/fd8ad97824324fe2b74c87a3d58bb526?sid=3539c3ff-46ff-4d26-a5d4-e1e9dbc44541

Version

cloud

ichik commented 3 months ago

Seems like we're propagating changes to tokens in two steps rather than applying them everywhere at once? @KelvinOm @riodeuno @jsartisan @marks0351 anything that we could do about it?

jsartisan commented 3 months ago

I think the visual distortion is expected no? 🤔 Since different fonts have different character widths which is causing the width of widget change on font change. I am not sure what is the expectation here? @ichik @ramsaptami

ramsaptami commented 3 months ago

@jsartisan there's 2 parts to the video, first bit shows behaviour on the widget selection DP & second half on the previous version of the base DP - you'll notice the transition is smoother on the latter.

ichik commented 3 months ago

@jsartisan it's about two reflows that happen. If you watch the video closely you can see that adjustments happens in two steps after typeface change.

KelvinOm commented 3 months ago

Related conversation https://github.com/appsmithorg/appsmith/pull/33374#discussion_r1598233397

ramsaptami commented 3 months ago

@jsartisan please add estimates if we're working on this; we can move this to the backlog if this isn't being picked up right now too

KelvinOm commented 3 months ago

@ramsaptami Moved task to new issues list because it's low priority and we'll postpone it for a while.