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

Fixed truncation of webhook url to avoid disappearance of Trash Icon #4145

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

JodhwaniMadhur
Copy link

resolved trash icon disappearance when the webhook url is too long.

@JodhwaniMadhur
Copy link
Author

hello @boojack , I have made the changes, but please tell me how to resolve this such that the pipeline doesn't fail.

@RoccoSmit
Copy link
Contributor

@JodhwaniMadhur in your IDE run pnpm run lint and it will show you where the linting error is taking place. You can also run pnpm run lint --fix to let the linter have a go at fixing the issue for you.

It does look strange that it wants you to have so many spaces. It might want the TD content on a separate line

@JodhwaniMadhur JodhwaniMadhur marked this pull request as ready for review November 24, 2024 11:00
@RoccoSmit
Copy link
Contributor

RoccoSmit commented Nov 24, 2024

Existing:
image

PR:
image

Can the Name column be resized to match the original size. The new design does not allow for the full URL to be viewed so the more space we can give to the URL column the better.

Suggestion:
With the scrollbar now being removed, can we add a title to the td so that we can see the full path on hover (similar to what is done with memo dates)

image

@JodhwaniMadhur
Copy link
Author

#4145 (comment)

can we address this in a new feature? and ofc, I would like to take it up as usual.

@RoccoSmit
Copy link
Contributor

RoccoSmit commented Nov 24, 2024

#4145 (comment)

can we address this in a new feature? and ofc, I would like to take it up as usual.

I would consider a way to view the full url as part of this PR as this functionality exists and is being removed by keeping the delete button visible. I added a suggestion.

With being able to view the URL the spacing is not an issue

Copy link
Contributor

@RoccoSmit RoccoSmit left a comment

Choose a reason for hiding this comment

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

LGTM

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