-
-
Notifications
You must be signed in to change notification settings - Fork 42
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: Enable viewing all workflow form sections at once #2310
Conversation
8775a40
to
98cf1b4
Compare
98cf1b4
to
f542e60
Compare
This comment was marked as resolved.
This comment was marked as resolved.
6606398
to
be9fac8
Compare
9256579
to
b69804e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, left a few little suggestions! The biggest issue I found here was that the intersection observer currently only made changes when an element went from not visible to visible, which meant that the "Limits" section was never focused at moderate viewport sizes because it starts out already partially visible. The suggestion I made adds a Set
of visible panels that gets updated any time panels come in or out of the viewport, and then sets the active panel to the first visible panel instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setup guide link isn't visible when editing, it also seems to be broken (doesn't open the sidebar).
Other than that, looks great! I like the drop shadow for the selection state.
I noticed it's not opening as well, reported in #2335 We never had it enabled in edit, but I'm unsure why, and it makes sense to me to show it in edit as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oop, missed that! Looks good to me then! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, nice work!
Resolves #2169
Changes
<btrix-observable>
into newObservable
controllerManual testing
**
.) Verify form submission is prevent and form is scrolled to the errored fieldScreenshots