-
Notifications
You must be signed in to change notification settings - Fork 283
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
Fixes #7, another dormant bug #9
Conversation
@@ -26,7 +26,8 @@ export default React.forwardRef((props, ref) => { | |||
</Link> | |||
); | |||
} else { | |||
const href = absolutePath ? url.toString() : cleanDuplicateSlash(getBasename() + url.toString()); | |||
// Note: + '/' + is required here | |||
const href = absolutePath ? url.toString() : cleanDuplicateSlash(getBasename() + '/' + url.toString()); |
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.
You add 1 more slash then replace it. Does it make sense?
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.
getBaseName()
can return either
/abc/def
, or /abc/def/
We would have to cover both scenarios
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.
Those two options are possible because we leave the work of filling the homepage
field, to the user.
The user can enter the homepage url with a trailing slash, or not... It's up to them, and we should make sure things work both way
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.
Looks good to me.
* Initial commit to add bulk workflow deletion * Added an API endpoint for bulk deletion * Add archiveworkflow param * Added bulk delete action to BulkActionModule * Added archive option to bulk delete * Addition of terminateRemove() Functions (#9) * Added an API endpoint for bulk deletion * Fixed WorkflowBulkServiceTest, added test cases for deleteWorkflow an… (#10) * Fixed WorkflowBulkServiceTest, added test cases for deleteWorkflow and terminateRemove, and added WorkflowResourceTest test for single terminateRemove * pass spotlessjava --------- Co-authored-by: JeffP <jeffp@jeffpham.com> --------- Co-authored-by: jeffp1 <jeffp@jeffpham.com> Co-authored-by: Ayush Thengne <ayushthengne@ayushs-macbook-pro.local> Co-authored-by: Jeff P <28172529+JeffP07@users.noreply.github.com> Co-authored-by: Doe1111 <106997600+Doe1111@users.noreply.github.com>
* Fixes conductor-oss#7, another dormant bug * Review comment - Combine Regexes
* Fixes conductor-oss#7, another dormant bug * Review comment - Combine Regexes
Pull Request type
./gradlew generateLock saveLock
to refresh dependencies)NOTE: Please remember to run
./gradlew spotlessApply
to fix any format violations.Changes in this PR
Describe the new behavior from this PR, and why it's needed
Issue #7
Alternatives considered
Describe alternative implementation you have considered