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.
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: readme page #4145
feat: readme page #4145
Changes from 21 commits
520b28a
e580475
a74aa58
a1383d7
50fbea7
0f79dc4
188ecaa
135cc46
59f6dc1
6408eaf
f389f3c
ad1fd62
a9521ec
4d56ed4
6fb504a
84f8b77
7fde819
685aad6
20ff912
945e8e7
fff13fa
36f9dee
4cba8ec
b2bec23
92450d2
16cc688
4678e0e
1e2c8c8
987e3e0
91706bb
1a85cd8
a827afe
7bbd66b
0d6bca8
e5e9c0d
78c6dd3
4de0d58
c90d4b0
fbfd644
5c61b9f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
There's a
multiline
prop for this component which we should use for both of theseRichTextInput
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 otherRichTextInput
s - 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 themultiline
prop is present - will add to Trello(I've added the prop here in any case).
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.
I think the
rows
prop is what we're after here, for example -https://github.com/theopensystemslab/planx-new/blob/main/editor.planx.uk/src/pages/GlobalSettings.tsx#L121-L132
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.
Ah great, thanks!
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.
Although it still doesn't make the text area larger by default 🤔 Will keep digging!
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 theRichTextInput
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 👍
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 👍
This file was deleted.