feat: readme page #4145
feat: readme page #4145
Conversation
|
Removed vultr server and associated DNS entries |
|
🤖 Hasura Change Summary compared a subset of table metadata including permissions: Updated Tables (2)
|
jamdelion
force-pushed
the
jh/experimental-readme-page
branch
from
a339586 to
59f6dc1
Compare
hasura.planx.uk/migrations/1737026490001_alter_table_public_flows_add_column_summary/up.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Copied from Ian's branch: #4120
There was a problem hiding this comment.
A few comments on code - no show stoppers.
Everything was super clear and easy to review - a pleasure! Apologies again it took me so long to get to this one.
One thing to note on a design level is that this doesn't match other similar setting pages, with a card per grouped input, e.g. Design or Settings.
| Settings page | Readme page |
|---|---|
 |
 |
These are constructed with the SettingsForm component which may help keep things standardised and consistent, both visually and in terms of user experience.
hasura.planx.uk/migrations/1737026490001_alter_table_public_flows_add_column_summary/up.sql
Outdated
Show resolved
Hide resolved
| Save | ||
| </Button> | ||
| <Button | ||
| onClick={() => window.location.reload()} |
There was a problem hiding this comment.
Is there a less drastic option like resetting the form we could go for here? I guess a simple formik.resetForm() won't work as there are multiple fields, some of which may have updated.
Maybe for each unupdated (dirty) field, we can revert to the initial value.
Please see comment in main review - maybe reaching to SettingsForm could help us out here.
There was a problem hiding this comment.
Thanks Daf - the problem here was that because the latest data is fetched when the route loads, we get unexpected behaviour if the reset button reverts to the initial value after a user has saved their changes. I.e. it reverts back to before their saved changes instead of accounting for them, even though they have been updated in the background correctly.
Perhaps a solution would be to refetch the data before calling formik.resetForm once the reset button is clicked 🤔
There was a problem hiding this comment.
Yeah we're bumping into the awkward data caching / fetching issue we talked about that tanstack query would resolve.
We probably could reset values for formik.initialValues on success but this is a little janky!
This solution works well for now for sure - I think the reset is a bit of an edge case - and lets make a proper plan for how we could integrate tanstack query 👍
| <>What does this service not include?</> | ||
| </SettingsDescription> | ||
| <InputRow> | ||
| <RichTextInput |
There was a problem hiding this comment.
There's a multiline prop for this component which we should use for both of these RichTextInput fields.
It's recently come up as a usability issue when we allow rich text but only provide a small one line input.
There was a problem hiding this comment.
Thanks I was wondering about this - with the multiline prop the default text area of the input doesn't change, and I've checked instances where the prop is added to other RichTextInputs - e.g. the editor for upload and label and those also still have a single line input. Whether the prop is there or not, the input expands when lots of text is added though.
I do think I'd expect this text box to be large by default though - we probably need to add something so that RichTextInput changes style when the multiline prop is present - will add to Trello
(I've added the prop here in any case).
There was a problem hiding this comment.
I think the rows prop is what we're after here, for example -
There was a problem hiding this comment.
Ah great, thanks!
There was a problem hiding this comment.
Although it still doesn't make the text area larger by default 🤔 Will keep digging!
There was a problem hiding this comment.
E.g. the globalSettings line corresponds to this in the UI - i.e. still appears as a single line input
There was a problem hiding this comment.
Yep very good spot! There's a bug / issue here where the MUI input props (rows, multiline) are not getting correctly transposed onto their tiptap equivalents in the RichTextInput component.
It would be nice to come back and address this but it's outside the scope of this PR and currently behaves exactly as the rest of the Editor does 👍
Thanks Daf - I have compared my form and the |
🎟️ Trello ticket
In this PR:
flowSummaryandflowLimitationsinflowstableflowStatusattribute to populate the flowTag.FlowTagdefinition from @ianjon3s's work here: wip: Team page filters and cards #4120flowDescription,flowSummaryandflowLimitationsfor now.teamViewersNB:
resetbutton refreshing the page, but this was the quickest way to eliminate a bug that Silvia encountered without adding in another API call (since the latest data is fetched when the route loads).flowstable for most user roles. I noticed that it wasn't enabled for demo users though, so I added it there. 🧼disabledvs a normal textInput. The latter goes grey and prevents typing, whereas the former remains usable at the moment.Still to do (coming up in next PR):
Screenshot