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

Delete Scale UI #528

Open
wants to merge 4 commits into
base: lyra2019
Choose a base branch
from
Open

Delete Scale UI #528

wants to merge 4 commits into from

Conversation

NanaKwame
Copy link
Contributor

Adding UI for deleting scales.

@NanaKwame NanaKwame linked an issue Nov 24, 2020 that may be closed by this pull request
@NanaKwame NanaKwame requested a review from ktbacher November 24, 2020 17:32
@NanaKwame NanaKwame added the 🚧 WIP Work in Progress PRs label Nov 24, 2020
@lgtm-com
Copy link

lgtm-com bot commented Nov 24, 2020

This pull request introduces 1 alert when merging 2486672 into 1a55132 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@NanaKwame
Copy link
Contributor Author

FYI in guidesReducer.ts, guideReducer deletes guides when scales are deleted.

@NanaKwame NanaKwame removed the 🚧 WIP Work in Progress PRs label Nov 24, 2020
@lgtm-com
Copy link

lgtm-com bot commented Nov 24, 2020

This pull request introduces 1 alert when merging 3637882 into 1a55132 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@@ -109,7 +143,12 @@ class ScaleList extends React.Component<StateProps & DispatchProps> {
data-scale={scale.get('_id')}
data-field={field}>
{/* <ContentEditable value={name} save={updateScaleName} /> */}
{name}
<div style={{ "marginLeft": "26px" }}>
Copy link
Member

@jonathanzong jonathanzong Dec 3, 2020

Choose a reason for hiding this comment

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

let's try to avoid inline styles if possible. can we put this in the stylesheet?

@jonathanzong
Copy link
Member

jonathanzong commented Dec 3, 2020

this looks good! requested behaviors:

  • when deleting a color scale, the marks disappear. if the encoding is "fill" can we replace the value with a default instead of dropping the encoding?
  • after deleting a size scale, manually changing the size using the inspector no longer works

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.

UI to Delete Scales
3 participants