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.

Sign up for GitHub

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

Jump to bottom

Fix bug report confirmation dialog width and scroll #6573

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Fix bug report confirmation dialog width and scroll #6573

merged 4 commits into from
Sep 24, 2024

Conversation

Parthiv-M
Copy link
Contributor

@Parthiv-M Parthiv-M commented Sep 19, 2024 •
edited by thecodrr
Loading

  • EDIT: Previously, the message returned a string enclosed in `` with fixed spacing/new lines. This has been changed to be concatenated with + s so that a `

    ` is generated instead of a `pre` which was causing the fixed width and ugly scroll issue

  • Fixes "Thank you for reporting" dialog cuts out text #5559

@Parthiv-M
web: modify message to return <p /> instead of <pre />
003dfd9 
to fix width and scroll of bug submission confirmation dialog

Signed-off-by: parthiv-m <[email protected]>
@Parthiv-M
web: rollback changes made for testing
a518353 
Signed-off-by: parthiv-m <[email protected]>
@thecodrr
Copy link
Contributor

This makes no sense. `` doesn't produce pre tag. It's just used for string substitution. Changing it to "" doesn't make a difference. Have you actually tested this change?

thecodrr
thecodrr requested changes Sep 23, 2024
View reviewed changes
apps/web/src/dialogs/issue-dialog.tsx Outdated
Comment on lines 153 to 157
message: "You can track your bug report at [" + url + "](" + url + ")."

+ "\n\n<br />Please note that we will respond to your bug report on the link above. **We recommended that you save the above link for later reference.**"

+ "\n\n<br />If your issue is critical (e.g. notes not syncing, crashes etc.), please [join our Discord community](https://discord.com/invite/zQBK97EE22) for one-to-one support."
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unnecessary. The pre tag is created due to the indentation. Just remove that and it'll work.

To add empty line breaks in Markdown, use \\ instead of <br>.

@Parthiv-M
Copy link
Contributor Author

This makes no sense. `` doesn't produce pre tag. It's just used for string substitution. Changing it to "" doesn't make a difference. Have you actually tested this change?

Yep, it worked in my tests (locally). But I'll make the indentation change nevertheless.

@Parthiv-M
Copy link
Contributor Author

Hi, I realised that the way I interpreted the change was wrong, rather than the usage of backticks itself. The code that fixed it was the concatenation of strings rather than removing interpolation altogether.

I have reverted back to using backticks but concatenated them instead of giving fixed spacing.

My apologies for the confusion. I have attached a screenshot of the result and updated the PR description too.

bug_report

@thecodrr thecodrr merged commit 0ff38c7 into streetwriters:master Sep 24, 2024
1 of 3 checks passed
@thecodrr
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thecodrr thecodrr thecodrr requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Thank you for reporting" dialog cuts out text
2 participants
@Parthiv-M @thecodrr