Skip to content
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

chore: Move share button #3103

Merged
merged 24 commits into from
Jan 11, 2024
Merged

chore: Move share button #3103

merged 24 commits into from
Jan 11, 2024

Conversation

timarney
Copy link
Member

@timarney timarney commented Jan 9, 2024

Fixes: #3059

  • Moves save button out of left navigation
  • Adds save button to translate page
  • Adds save error toast message (also works for auto-save)
Screenshot 2024-01-10 at 2 58 41 PM Screenshot 2024-01-10 at 3 09 37 PM Screenshot 2024-01-10 at 3 14 04 PM Screenshot 2024-01-10 at 3 14 52 PM

Copy link
Contributor

github-actions bot commented Jan 9, 2024

@timarney timarney force-pushed the chore/3059-save-button branch 6 times, most recently from e95b718 to 54cc226 Compare January 10, 2024 14:37
@timarney timarney marked this pull request as ready for review January 10, 2024 19:55
@thiessenp-cds
Copy link
Contributor

One minor change. It would be helpful to have focus placed on the "Failed - Contact support" after the error is shown. Currently looking to see how difficult that would be

@thiessenp-cds
Copy link
Contributor

That could. be difficult. I think we need to refactor StyleLink to receive a ref for the error link. Then in a useEffect, if there is an error, focus that ref. There may be other ways. If we were to go this route, the code change may impact other parts of the app, so probably best to do outside this PR

thiessenp-cds
thiessenp-cds previously approved these changes Jan 10, 2024
Copy link
Contributor

@thiessenp-cds thiessenp-cds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to revisit the Toast for this and similar use cases in a future task. Also the the comment about focussing the error link but as mentioned, that's probably best to do in a separate PR.

Nice work, LGTM :)

@timarney
Copy link
Member Author

@thiessenp-cds I attempted to use a ref + use effect on the title inside the toast notification but it didn't seem to be working. Agreed we could do some further work under a future PR.

@timarney timarney changed the title Move share button chore: Move share button Jan 11, 2024
@timarney timarney enabled auto-merge (squash) January 11, 2024 15:50
Copy link
Contributor

@thiessenp-cds thiessenp-cds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, LGTM :)

@timarney timarney merged commit 89c1909 into develop Jan 11, 2024
11 checks passed
@timarney timarney deleted the chore/3059-save-button branch January 11, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move "Save Draft"
4 participants