EnxDev commented on code in PR #45337:
URL: https://github.com/apache/superset/pull/45337#discussion_r4282824391
##########
superset-frontend/src/pages/ActionLog/index.tsx:
##########
@@ -48,6 +50,15 @@ export type ActionLogObject = {
const PAGE_SIZE = 25;
+const JsonCellTrigger = styled.div`
+ [data-test='span-modal-trigger'] {
Review Comment:
Good catch, thanks. You're right, `babel-plugin-jsx-remove-data-test-id`
drops `data-test` in production, so the ellipsis and cursor would have only
worked locally.
Switched the selector to `> [role='button']`, which is what ModalTrigger
renders on the trigger div and survives the production build. I also added a
test that strips `data-test` from the trigger before checking `white-space` and
`text-overflow`; it fails against the old selector and passes with the new one.
Fixed in 2a65556.
##########
superset-frontend/src/pages/ActionLog/index.tsx:
##########
@@ -215,24 +226,42 @@ function ActionLogList() {
row: {
original: { json },
},
- }: any) => (
- <Typography.Text
- css={css`
- .ant-typography-copy {
- visibility: hidden;
- }
- &:hover .ant-typography-copy {
- visibility: visible;
- }
- `}
- copyable={!!json}
- ellipsis={{
- tooltip: { styles: { root: { maxWidth: '900px' } }, title: json
},
- }}
- >
- {json}
- </Typography.Text>
- ),
+ }: any) => {
+ const jsonObject = safeJsonObjectParse(json);
Review Comment:
I'm going to leave this one as is. The list is capped at 25 rows per page,
and `safeJsonObjectParse` bails out on the first character for anything that
isn't an object or array. The payloads themselves are logged request arguments
(paths, rison params, impression data), so they're small. SQL Lab result cells
already run the same parse per cell on much larger result sets.
If we ever see oversized log payloads in practice, moving the parse into the
click handler would be easy, but I don't think it's worth the extra code right
now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
