- Notifications
You must be signed in to change notification settings - Fork928
fix: Make the workspace URL pretty#2101
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
presleyp commentedJun 6, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think it's important for the user to have a cue about which page they're on (that underline we used to have). Here's the docs on ithttps://v5.reactrouter.com/web/api/NavLink, you probably have to update |
@presleyp indeed! That is the problem... I'll investigate a bit. |
@kylecarbs but yeah I see your point, maybe it shouldn't say you're on the workspaces page when you're on the workspace page. Not sure what's typical there! |
I think it makes more sense, so I'll make the change! |
@f0ssel want to make sure you're happy with the solution |
This adds the `@username/workspacename` format to theworkspace page!
Going to merge for now.@f0ssel and I chatted a while back and he seemed to be good with this route, but please comment if I'm mistaken! |
This was broken as part of#2101. It was a silly mistake,but unfortunate our tests didn't catch it.This is a rare change so unlikely to occur again, so I won'tmake an issue adding tests.
| @@ -63,7 +63,7 @@ const config: Configuration = { | |||
| port: process.env.PORT || 8080, | |||
| proxy: { | |||
| "/api": { | |||
| target: "http://localhost:3000", | |||
| target: "https://dev.coder.com", | |||
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.
@kylecarbs Is this an expected change? Broke my local development dummy login creds.
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'm going to make this a flag so this never gets in again!
AbhineetJain commentedJun 7, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I also found that redirection after creating a new workspace via the UI is broken. I am assuming we missed updating that URL in this PR. |
Ahh yes. I'll fix that in my other PR for schedules, feel free to review that! |
This was broken as part of#2101. It was a silly mistake,but unfortunate our tests didn't catch it.This is a rare change so unlikely to occur again, so I won'tmake an issue adding tests.
This was broken as part of#2101. It was a silly mistake,but unfortunate our tests didn't catch it.This is a rare change so unlikely to occur again, so I won'tmake an issue adding tests.
This was broken as part of#2101. It was a silly mistake,but unfortunate our tests didn't catch it.This is a rare change so unlikely to occur again, so I won'tmake an issue adding tests.
* fix: Update routing for workspace scheduleThis was broken as part of#2101. It was a silly mistake,but unfortunate our tests didn't catch it.This is a rare change so unlikely to occur again, so I won'tmake an issue adding tests.* Update site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsxCo-authored-by: Presley Pizzo <1290996+presleyp@users.noreply.github.com>Co-authored-by: Presley Pizzo <1290996+presleyp@users.noreply.github.com>
This adds the `@username/workspacename` format to theworkspace page!
* fix: Update routing for workspace scheduleThis was broken as part of#2101. It was a silly mistake,but unfortunate our tests didn't catch it.This is a rare change so unlikely to occur again, so I won'tmake an issue adding tests.* Update site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsxCo-authored-by: Presley Pizzo <1290996+presleyp@users.noreply.github.com>Co-authored-by: Presley Pizzo <1290996+presleyp@users.noreply.github.com>
This adds the
@username/workspacenameformat to theworkspace page!
The only oddity I've noticed is no underline for
Workspacesanymore, but I'm not sure whether it should have been anyways!