Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
fix: when deleting a workspace offline, the layout breaks #17639
fix: when deleting a workspace offline, the layout breaks #17639
Changes from 16 commits
6930885
9a094e3
e8e90e1
d07e3b1
cfb3b76
0240f90
4da28a3
732c034
3e532f7
58a96ff
4ca7979
75ab457
9f61f84
59bcbd9
3bcbdfb
cd1f55c
2d898b1
ae1ae33
9f9d54b
b56477e
56a90a7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
That is a weird bug.
Thanks for coming up with a fix so fast, but I wonder why this is happening and how could we fix this without applying workarounds.
I'll try to investigate this further a bit before we can merge this
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.
Sounds good. I have an additional fix incoming for this issue: #17656. Just need to verify the details first.
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.
@akinwale @eVoloshchak can you remind me please why is this change here?
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.
To summarise: Some of the menu items made use of the
openOldDotLink(url)
method, which built the URLs using the inner definedbuildOldDotURL
method. Since the original issue scope was to make the "Copy to clipboard" behaviour happen for all links, I had to refactor this method to the outer scope.You can follow the discussion here for more details: #17310 (comment)
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.
@akinwale Ok thank you, can you also add a Qa test which will check the deeplink works as expected, also tehre is a merge conflict, I will merge once this is resolved, thank you!
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.
@mountiny All done!