Skip to content

Web27 Display Filename #347

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

Merged
merged 10 commits into from
Jun 11, 2023
Merged

Web27 Display Filename #347

merged 10 commits into from
Jun 11, 2023

Conversation

winnteas
Copy link
Contributor

@winnteas winnteas commented Apr 6, 2023

Why the changes are required

  • to see what file is being edited on the editor page

Changes

  • passed in filename when navigating to the editor page (via edit button or double clicking)
  • filename shows on the left side of the header, buttons remain on the right side

image

jamest0
jamest0 previously requested changes Apr 6, 2023
Copy link
Member

@jamest0 jamest0 left a comment

Choose a reason for hiding this comment

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

lgtm but please fix the frontend_testing pipeline before merging - Error: Uncaught [Error: useLocation() may be used only in the context of a Router component.]

  • you probably need to wrap the parent element in the Router component.

@jamest0
Copy link
Member

jamest0 commented Apr 6, 2023

I think you can use a parent useState in Dashboard.tsx and pass it to the Renderer and Sidebar so that you can pass the name to the Edit button too.
Similar to how the selectedFile / setSelectedFile is implemented.

Copy link
Contributor

@lauraw0 lauraw0 left a comment

Choose a reason for hiding this comment

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

just some style changes needed but besides that lgtm! you can merge once you have removed those unused imports

@winnteas winnteas merged commit e4c0654 into main Jun 11, 2023
@winnteas winnteas deleted the WEB27-display-filename branch June 11, 2023 12:28
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.

3 participants