-
Notifications
You must be signed in to change notification settings - Fork 11
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
inline form editing update #38
Open
pjm17971
wants to merge
16
commits into
master
Choose a base branch
from
form-change-indications
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cleanup of the forms list editing UI Moves icons back to the right Removes the edit list followed by toggle to edit item flow. Instead you can just hover on a list and click the edit (or delete or new item) Moves the edit icon for the text edit to the right, using the widget itself to render it so it can appear right next to the text. Adds header to the list view, which you can control via a static member on the item's form.
Fixes the textedit input to be semi-controlled. That is, if the input is focused it is uncontrolled. If it is not focused it is driven by the value prop.
New inline editing style. Component is now properly controlled. Added random text to the notes field in the basic form example Added lines above and below the text area
Refactor styling of view renders TextEdit doesn't show edit action after blur Inline editing of radio buttons and check boxes re-enabled Inline editing of chooser Chooser opens and focuses when inline edited
Two main aims here: - move the inline editing pencil icons to the right of the control's view. This is makes the editing experience similar to other such forms. The functionality for those is now pushed to the controls themselves, so they can place those depending on the control itself. This also will be analogous to the way inline editing of lists will show edit and delete icons to the right of the list item. - Completing of an inline form can now be with a Done button (or press enter) or can be aborted with the Cancel button (or press esc). Different controls had different ways of reacting to a blur so the experience was mixed. This way there's two clear actions the user can take. This will also be the case for editing a list item.
First pass on fetching options for the chooser in an async manner. Constraints: 1) This is a new component AsyncChooser, because of the differences I'm about to describe. All the props are the same, except for how options are supplied (sort of obviously), so migration for those chooser should be relatively painless. You supply a `loader` prop, which is a function, rather than supplying a list of options (kind of the point). 2) You need to supply the `value` as an Immutable.Map that includes both the label and id e.g. `Immutable.Map({ id: 2, label: "Argonne National Lab"})`, vs previously `2`, which means this needs to be available in the data already loaded on the page. The reason for this is if you just passed in an id as before there's no data to look up the label until it loads later. This is kind of annoying on the regular form because the existing value wouldn't show up until the contents of the chooser were loaded, but it's really fatal in the view/inline version of the form, because there's no way to even display the label at all. So, you need to come up with that. If that's not possible in ESDB somewhere, I'd probably propose either not using the AsyncChooser there, or modifying the backend so it can supply both of those for the current value. I suspect you have that though. 3) You can't change the option list after it's loaded and cached internally. Before you could change the options prop. Since it goes and fetches the data and internally caches, you can't do that now. 4) It might be possible with the way things are arranged (in the future) to also do incremental loads based on what the user types, but I don't think our backend is anywhere ready for that, so that's not part of how this works yet. It expects you to fetch it all. But the result is I think basically what we want: - In the full form, all AsyncChoosers will "spin" as they fetch their data. So you'll see most of the form, but to select something from a loading chooser you'd have to wait for it to stop spinning. That's a bit of backward step for the user, except they saw most of the form quicker, so it might be better. Ultimately we still need to load the data. - In the view/inline editing case no data is loaded until the user tries to edit that chooser. That should make loading much faster for those pages, which I assume is our 90% case The other thing to maybe mention is the `loader` callback is probably not really going to work with apollo client. I mean it will work along side it, but it won't use it. It's kind of imperative, vs the declarative approach of apollo client. But I think we can live with that for the performance gains. I don't really know in what way we could make it work better in that regard, but maybe it will become clearer when we're much further down the graphql path. So, you'll probably just make a fetch() call in the `loader` callback for the graphql or REST that has the options in it and return the result to the chooser. Here's a bit of code showing how it's used ``` <AsyncChooser field="color" width={300} loader={this.colorLoader} /> ``` And the colorLoader: ``` colorLoader(input, cb) { setTimeout(() => { cb(null, availableColors); }, 2000); } ``` The first arg to the `cb` is an error, though I don't really display that yet. The second is the Immutable.List of {id, label} dicts (which is what was passed in before as the available options). So basically in there, where there's the setTimeout, you'd do your fetch for the list and when you get the result, process it to the right format and send it back to the control with the cb call.
feat: Block inline commit if there's an error or missing value
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why move them? Most inline editing has edit actions to the right, so more familiar. It also enables multiple icons without taking up increasing space between the label and the control. This means the list editing can be displayed the same as other inline editing, but display both a row edit icon and a delete row icon together. Plus, we can add a revert icon.