Use raw number for number column overlay and copy #8708
Conversation
LukasMasuch
changed the title
LukasMasuch
added
type:enhancement
LukasMasuch
added
change:feature
and removed
type:enhancement
LukasMasuch
changed the title
|
Could you please add a screenshot of before and after to make it really easy to spot the difference in the dataframe (the demo only shows the new behavior)? |
There isn't any change in the table rendering. But there is a small visible change in the overlay editor for which we don't have any e2e tests yet. I'm currently attempting to build something to e2e test this, but not sure how much effort it is. |
…n-thousand-separator
….com/streamlit/streamlit into fix/number-column-thousand-separator
|
@raethlein I added additional e2e tests to test the visuals in the overlay editor within the + migrated the expect written text method to |
| from playwright.sync_api import Locator, expect | ||
|
|
||
|
|
||
| def expect_prefixed_markdown( |
There was a problem hiding this comment.
thanks for extracting the utils functions. I hoped that it could also be used for the expect_written_text function in the st_altair_chart_basic_select function here, so I believe it would make sense to offer it directly in a way where it'd be compatible. But can also be done in a future PR
There was a problem hiding this comment.
Updated st_altair_chart_basic_select to use the utils method 👍
| """ | ||
| # This is currently the best way to get the cell overlay | ||
| # We should eventually add a stable test ID to the cell overlay | ||
| # within GDG to better target it. |
There was a problem hiding this comment.
nit: "GDG" -> glide data grid. I expect no one very familiar with the matter to know the abbreviation
| ) | ||
|
|
||
| cell_overlay_test_column_config = { | ||
| # the e2e test requires all cells to medium width |
There was a problem hiding this comment.
nit: could you add a short comment why the e2e test requires this, please?
|
@raethlein Ready for another round |
Describe your changes
This PR enforces that the raw number value is used in the cell overlay and as copy value without any formatting applied. This was always supposed to be the case, but wasn't fully enforced in the number column yet.
Demo
GitHub Issue Link (if applicable)
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.