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

Toggleable fields #22

Merged
20 commits merged into from
Oct 25, 2022
Merged

Toggleable fields #22

20 commits merged into from
Oct 25, 2022

Conversation

ghost
Copy link

@ghost ghost commented Oct 23, 2022

This PR is a preparation for us to support advanced fields with nested options and introduces the following changes:

  • New columnar layout
  • Toggleable fields
  • Bin option
CleanShot.2022-10-23.at.01.02.43.mp4

@josevalim
Copy link
Contributor

Functionality wise, it looks great to me, I only have concerns about the layout since we have too many boxes now.

I think we should probably use a different design inside the box. Today we have this:

x-axis
[                       v]
--------------------------
| Type                   |
| [ quantitative      v] |
|			 |
| Aggregate        Bin   |
| [            v]  [   ] |
|			 |
--------------------------

and I would go with something like this:

x-axis
[                           v]
------------------------------
| Type        quantitative v |
| Aggregate                v |
| Bin                  [   ] |
------------------------------

I will try to submit a mockup soon.

@josevalim
Copy link
Contributor

Here is a mockup:

Screenshot 2022-10-23 at 12 34 26

But I would consider removing the select border altogether:

Screenshot 2022-10-23 at 12 43 24

Here is the CSS I used but I was doing those changes in the browser, perhaps some of those deserve to be new classes altogether:

.field-settings-container {
  padding: 4px 10px;
  border-radius: 0.5rem;
  margin-bottom: 4px;
  border: 1px solid var(--gray-200);
}

.field-settings-container .field {
  margin-bottom: 4px;
}

.field-settings-container .field:last-child {
  margin-bottom: 0;
}

.field-settings-container .input-label {
  margin-bottom: 0;
}

.field-settings-container select {
  border: 0;
  padding-right: 16px;
  background-color: transparent;
  background-position: center right;
}

.field-settings-container select:focus {
  border: 0;
}

@ghost ghost requested review from jonatanklosko and josevalim October 24, 2022 21:18
@ghost ghost force-pushed the cg-toggle-fields branch from 33eee12 to cd7a576 Compare October 24, 2022 21:22
Copy link
Member

@jonatanklosko jonatanklosko left a comment

Choose a reason for hiding this comment

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

Awesome :)

@josevalim your call about the UI. I think the boxy feeling is mostly because we have the smart cell border close to the options border, but I don't have cleaner alternative in mind right now.

@ghost ghost merged commit 923c4f1 into livebook-dev:main Oct 25, 2022
@ghost ghost deleted the cg-toggle-fields branch October 25, 2022 00:17
This pull request was closed.
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.

2 participants