-
-
Notifications
You must be signed in to change notification settings - Fork 386
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
Add a "new record" button at the bottom of grid and detail views #1431
base: main
Are you sure you want to change the base?
Add a "new record" button at the bottom of grid and detail views #1431
Conversation
Deployed commit |
9ca236c
to
337c725
Compare
viewInstance.viewPane, | ||
dom.domComputed(use => | ||
(use(viewInstance.viewSection.hasFocus) && use(viewInstance.enableAddRow)), | ||
(showNewRecordButton) => | ||
showNewRecordButton ? newRecordButton(viewInstance) : null | ||
) |
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.
This is out of the viewPane
mostly because some views have overflow: hidden
on their wrapper and since we want the button to be a bit out of the view it needs to be outside the wrapper.
Deployed commit |
337c725
to
b4953db
Compare
Deployed commit |
Also, I'm unclear as to what the different between 1/ the new "Add card" button and 2/ the old "+" on top of the card view is It seems to me that they do slightly different things (1/ creates a new empty record whereas 2/ doesn't quite yet create the record but gives you the option to do it), but I don't know that they should ? 🤔 |
Yes the behavior is, for now, purposefully different (see my questioning part). I feel like for the card view it should stick to always doing the original behavior (number 2). But on the grid view current behavior is better (number 1)? |
I think the 'add new record' button is useful if you've got lot of lines (so in lot of cases), we keep the actual behavior too so when you're on a new table, i you're filling a first line , a new one is automatically created, so in that case you don't need to use the'new record button' but it's not a problem because they're two ways to do it |
I'm not fully sure about the actual behavior with the actual '+' (for exemple a 'add record' appear and user doesnt know if it's a button or a title, and the '+' is not very visible) , maybe adding 'add new record' button may sound not very useful for the card view but if user use 'add new record' for table view to create 5 new empty lines for example , he will find 5 new empty cards too if he's also using a card view, so better to let him the possibility to create it on the card view with a button |
I added a couple of our templates to the preview, to make it easier to play around with slightly more realistic data:
One early thought: when you select a contact in the CRM example, the common next step is to add a new interaction, but there is no indication how to do it -- no "New record" button for interactions. One still needs to know to click into that widget (which means clicking on an existing record, when there are many rows) just to see the option to add a new record. Another thought is that in a list+card arrangement as in the CRM example, adding a new contact in the table would normally be followed by entering the contact data in the linked card. In a custom app, the natural flow would be a page-wide "Add Contact" button, which focuses the Card widget every time. This is a vague thought, but perhaps someone has ideas. I think the unifying idea of these observations is that when there are linked widgets, the behavior of "New record" should ideally depend on the shape of linking. |
This makes sense to me. Anecdotally, I've seen people use the "Action Button" custom widget for this exact purpose (create a new record in a way that makes sense given the page that you are in, e.g. "add a new contact"). |
What this action button look like ? (apart a github folder?) |
@lusebille See this worflow management example I am working on: Beware: it is still in development! |
@manuhabitela , for the current implementation, I think it addresses only a subset of the confusion people have, and in some cases creates new confusions. So we wouldn't be comfortable releasing it to production as a default behavior. But doing user-testing and getting real user feedback on this would be great, so we are OK releasing it to production behind a URL parameter (as we discussed elsewhere, something simple like check We can then decide whether to keep this approach, if we need to give the user an option to turn it on or off, or if we want to do something different. |
Oh alright! What I had in mind after we talked about it a few weeks ago, and I think already started coding:
[1] What I was not sure, and don't remember, was if the user-feature stuff was already something completely coded, allowing to easily add new features tied to users, saved in their preferences. If not, I imagined I could just rely on localStorage and make it explicit in the modal that the feature would be enabled only on current machine. What do you think? |
We don't have such infrastructure. What do you think of a simpler approach: if a user opens a page with the special flag, the feature is on, and otherwise it's off? It wouldn't let users work normally with the feature, but it would allow an interview with a user working on their own doc for a session. Grist docs are single-page apps, so once a page is loaded, the JS state remains. So if you set a global variable like If you feel it's important to let users opt-in and persist this choice in the browser, I don't object, but it's more stuff to build and maintain, and I am not sure it will get any much use. |
I have to say I'm not that convinced with only keeping a variable in the state, since full page loads happen between documents list and document page, it quickly narrows the possibilities to what you just said: a specific testing session on one given document. I feel the dialog + keeping state in localstorage is not that much more code to maintain, stays rather simple (only client-side), and a good compromise between "only keep in js state for current doc" vs "keep it in user prefs server-side". It easily allows to globally share a link saying "hey, you can enable this experimental feature with this url and then work as usual on your docs to get a feel for it". So if you don't object I'd rather implement that. I actually checked earlier and I was almost done implementing the dialog + localstorage save, it's not that much code and it's pretty simple 😇 |
Sounds good! |
- add logic to enable the toggling of "experiments" via URL search params - first experiment of this sort is a "new record button" that is visible at the bottom left of a grid or detail view.
b4953db
to
ad9744b
Compare
@dsagal what do you think of this implementation? Note: tests tell me I missed a little something but I have to go right now 🤡 Here is a preview of what it does (@lusebille): grist-new-record-button.mp4 |
Context
See #366.
Lucie's mockups: https://www.figma.com/design/wcpetFt6aOKzTszcvPPWLQ/%5B05%2F24%5D-Grist-Design?node-id=3744-62552&t=MhknMbyFwd1RAc9n-0
Proposed solution
Very early wip that lead me to some questioning:
Conditions to show the button
The show/hide condition of the button certainly needs refinement. I mostly rely on the view's
enableAddRow
boolean, and show the button only when the view is focused, but that's pretty much it.Intended behavior on click
I'm not sure about the intended behavior.
I made it so that clicking the button actually adds a new record, and not just focuses the "new record" blue row. I felt it matched more what we would expect, at least in the grid view. Otherwise, clicking the "new record" multiple times would not actually add records but just stay focused on the last line.
But I feel this current behavior is a bit misleading when using the detail view. Maybe in that case we would want to just show the card with blueish inputs?
Button wording
I added the possibility to have different wording on the button depending on the view. For now it renders "New record" on the grid view and "New card" on the detail view.
I did that because it feels different wording for the detail view would maybe better, at least in some other languages than English. In French for example, translating "record" is a bit long and generic.
What do you think?
Related issues
Fixes #366.
Has this been tested?
Screenshots / Screencasts
Here is a preview:
Note: the preview is not up-to-date, as I updated the code to show we can have different wording for different views. Right now the button renders "new card" when focusing the detail view instead of "new record"
newrecord.mp4