Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

Implement number and range pickers. #1524

Merged
merged 29 commits into from
May 21, 2021

Conversation

MichaelMauderer
Copy link
Contributor

@MichaelMauderer MichaelMauderer commented Apr 26, 2021

Pull Request Description

Implements UI components for selecting number and a range of numbers according to #1199.

Peek.2021-04-26.09-45.mp4

Important Notes

  • Components not used anywhere yet.
  • Debug scene available at ?entry=slider. Shows a number of different possible configurations.
  • Adds a Component struct as a general building block for Frp/Model-based UI components.

Checklist

Please include the following checklist in your PR:

  • The CHANGELOG.md was updated with the changes introduced in this PR.
  • The documentation has been updated if necessary.
  • All code conforms to the Rust style guide.
  • All code has automatic tests where possible.
  • All code has been profiled where possible.
  • All code has been manually tested in the IDE.
  • All code has been manually tested in the "debug/interface" scene.
  • All code has been manually tested by the PR owner against our test scenarios.
  • All code has been manually tested by at least one reviewer against our test scenarios.

@MichaelMauderer MichaelMauderer added Category: BaseGL The BaseGL library Difficulty: Hard Significant prior knowledge requried Priority: High Should be scheduled as soon as possible Type: Enhancement An enhancement to the current state of Enso IDE Category: GUI The Graphical User Interface labels Apr 26, 2021
@MichaelMauderer MichaelMauderer self-assigned this Apr 26, 2021
@MichaelMauderer MichaelMauderer marked this pull request as ready for review April 26, 2021 11:10
Copy link
Collaborator

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

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

I really like the generalization of FRP component! However there are some solutions in the code I don't quite understand (and probably should be documented).

CHANGELOG.md Outdated Show resolved Hide resolved
src/rust/ensogl/example/src/slider.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/common.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/number.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/number.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/range.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/range.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/shadow.rs Outdated Show resolved Hide resolved
@MichaelMauderer MichaelMauderer requested a review from farmaazon May 4, 2021 19:39
src/rust/ensogl/lib/components/Cargo.toml Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/common.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/common.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/selector/common.rs Outdated Show resolved Hide resolved
Copy link
Member

@wdanilo wdanilo left a comment

Choose a reason for hiding this comment

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

  1. This looks absolutely stunning!
  2. I'm surprised of how much code is there!
  3. I'm not sure the proposed generalization is the right one (I explained in the comment) - Ill think about it.
  4. After watching the video 100 times (because I love it), I started missing one thing - a visual indicator on double range slider (maybe we should put it on other as well) - tell ing me "if you press, only right side will move) - something like highlight of the right edge, or the whole slider when moving mouse around. Would it be hard to add such an indicator ?

CHANGELOG.md Show resolved Hide resolved
src/rust/ensogl/example/src/leak.rs Outdated Show resolved Hide resolved
src/rust/ensogl/example/src/leak.rs Outdated Show resolved Hide resolved
src/rust/ensogl/example/src/leak.rs Show resolved Hide resolved
src/rust/ensogl/lib/components/src/component.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,134 @@
use crate::prelude::*;
Copy link
Member

Choose a reason for hiding this comment

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

no docs. Please check config why error was not emitted here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are no errors here (and in the other places noted) because none of this is public. All of this are utility modules that are not exposed as part of the selector module. So, they are public only within the selector module, but not outside it, and thus they are counted as not public.

@MichaelMauderer MichaelMauderer requested a review from wdanilo May 13, 2021 10:05
src/rust/ensogl/lib/components/src/component.rs Outdated Show resolved Hide resolved
src/rust/ensogl/lib/components/src/component.rs Outdated Show resolved Hide resolved
@MichaelMauderer MichaelMauderer merged commit b361fdb into develop May 21, 2021
@MichaelMauderer MichaelMauderer deleted the wip/mm/ide-1199-slider-component branch May 21, 2021 09:41
mwu-tow pushed a commit to enso-org/enso that referenced this pull request Oct 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Category: BaseGL The BaseGL library Category: GUI The Graphical User Interface Difficulty: Hard Significant prior knowledge requried Priority: High Should be scheduled as soon as possible Type: Enhancement An enhancement to the current state of Enso IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants