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

feat: Add resize drag handle to Dataset SQL fields #20670

Conversation

diegomedina248
Copy link
Contributor

SUMMARY

The SQL Editor in the Dataset modal doesn't have an option to resize.
Specially in the Metrics column, the field is way too small.

This PR enables resize in those fields.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Resize changes:

Screen.Recording.2022-07-11.at.10.40.24.mov

TESTING INSTRUCTIONS

Open the edit modal for a physical dataset and virtual dataset.
Ensure editor fields can be resized.

ADDITIONAL INFORMATION

Closes #20200, #14794

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

Merging #20670 (e252405) into master (6b0bb80) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #20670   +/-   ##
=======================================
  Coverage   66.83%   66.83%           
=======================================
  Files        1750     1750           
  Lines       65894    65900    +6     
  Branches     7017     7019    +2     
=======================================
+ Hits        44041    44047    +6     
  Misses      20067    20067           
  Partials     1786     1786           
Flag Coverage Δ
javascript 51.95% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...tend/src/components/Datasource/CollectionTable.tsx 81.35% <100.00%> (+0.48%) ⬆️
...end/src/components/Datasource/DatasourceEditor.jsx 65.87% <100.00%> (+0.13%) ⬆️
...rc/explore/components/controls/TextAreaControl.jsx 81.81% <100.00%> (+1.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b0bb80...e252405. Read the comment docs.

@@ -55,6 +65,8 @@ const defaultProps = {
maxLines: 10,
offerEditInModal: true,
readOnly: false,
resize: false,

Choose a reason for hiding this comment

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

resize is not of type boolean from the declaration above.

@@ -45,6 +45,16 @@ const propTypes = {
]),
aboveEditorSection: PropTypes.node,
readOnly: PropTypes.bool,
resize: PropTypes.oneOf([
null,

Choose a reason for hiding this comment

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

Are null and none the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not necessarily, cause none will actually set the css, whereas null will do nothing

@@ -335,6 +343,13 @@ export default class CRUDCollection extends React.PureComponent<
);
}

getCellProps(record: any, col: any) {
const cellPropsFn =
this.props.itemCellProps && this.props.itemCellProps[col];

Choose a reason for hiding this comment

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

this.props.itemCellProps?.[col] vs this.props.itemCellProps && this.props.itemCellProps[col]; ?

@zhaoyongjie
Copy link
Member

@diegomedina248 should rebase master to get rid of coverage failure.

@diegomedina248 diegomedina248 force-pushed the feat/add-resize-drag-handle-to-dataset-modal branch from 8787a88 to f22ebf6 Compare July 14, 2022 05:20
@zhaoyongjie
Copy link
Member

@diegomedina248 the master branch has a failure integration test, I have skipped it on the latest commit, and will fix it in separated pr. please rebase again. sorry for the multiple rebases.

@diegomedina248 diegomedina248 force-pushed the feat/add-resize-drag-handle-to-dataset-modal branch from f22ebf6 to e252405 Compare July 14, 2022 12:23
@rusackas rusackas merged commit dd353ca into apache:master Aug 2, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metric Tab of Dataset Edit Modal Does Not Have a Usable Experience
5 participants