Add localstorage saving to meeting report #4627
Conversation
jonasdeluna
force-pushed
the
temp-save-meetings
branch
from
fadc15c
to
78e9126
Compare
| @@ -451,6 +498,26 @@ const MeetingEditor = () => { | |||
| )} | |||
| </ConfirmModal> | |||
| )} | |||
| {isEditPage && | |||
| canDelete && | |||
| @@ -105,6 +105,8 @@ type MeetingEditorParams = { | |||
| }; | |||
| const MeetingEditor = () => { | |||
| const { meetingId } = useParams<MeetingEditorParams>(); | |||
| const [formDirtyFlag, setFormDirtyFlag] = useState(0); | |||
There was a problem hiding this comment.
Do you have any reasoning behind this naming? Didn't find it too intuitive
Also the method of refreshing the form is still kinda hacky - so might be nice with a comment about why this is needed.
| const [formDirtyFlag, setFormDirtyFlag] = useState(0); | |
| const [reportKey, setReportKey] = useState(0); // Needed to refresh the form component after loading the report content from localStorage |
Now I'm just throwing out ideas, but I'm assuming this prompt is only supposed to be shown at page load. Is there any neat way that you could prevent loading the report component until the prompt has been adressed? If you could do that and set the correct value before rendering the component, I imagine you wouldn't need the hacky key-changing
| const storedReport = sessionStorage.getItem( | ||
| `meeting-${meetingId}-report`, | ||
| ); | ||
| if (!storedReport || storedReport.length < newReportValue.length) { |
There was a problem hiding this comment.
Thoughts on doing this based on a timestamp rather than the length of the report? Even though this probably works in the majority of the cases, a longer report doesn't always mean a newer one
|
For me it does not appear to work properly.. When typing something into the report and then leaving the page without saving, I can see that it is saved to sessionStorage, however when I re-enter the meeting or meeting/edit I am not getting the prompt to get the data from sessionStorage. I also found that if something is added to sessionStorage I don't seem to be able to replace this. |
| `meeting-${meeting?.id}-report`, | ||
| ); | ||
| if (!storedReport || storedReport.length < reportValue.length) { | ||
| sessionStorage.setItem(`meeting-${meeting?.id}-report`, reportValue); |
There was a problem hiding this comment.
Minor/unlikely security concern. A user who has signed out after editing a report (e.g. on another persons computer) will still have this data in their session storage. A quick fix here is clearing it whenever a user signs out, i.e. add a sessionStorage.clear() here
There was a problem hiding this comment.
You can just clear it when this component unmounts
There was a problem hiding this comment.
I'm just adding it to logout button and then merging.
There was a problem hiding this comment.
Why add it to an action that has nothing to do with this?
jonasdeluna
force-pushed
the
temp-save-meetings
branch
from
78e9126
to
a6aa9ee
Compare
jonasdeluna
force-pushed
the
temp-save-meetings
branch
from
a6aa9ee
to
95a6007
Compare
Description
This adds localstorage saving to meeting reports.
Onchange, if the value is longer than the previous value it will override the localstorage value (or if the localstorage key does not exist).
This was re-opened as we decided at the meeting to go for this approach after consulting the router wiki.
Result
Caution
Make sure your images do not contain any real user information.
Testing
Resolves ABA-740