-
-
Couldn't load subscription status.
- Fork 17
Glasgow | Sheetal kharab | Module Legacy Code | WEEK 1| Hashtag slowing down my browser #28
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
base: main
Are you sure you want to change the base?
Glasgow | Sheetal kharab | Module Legacy Code | WEEK 1| Hashtag slowing down my browser #28
Conversation
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.
great work on fiing the issue! HOWEVER , you need to e careful NOT to introduce code changes that are not related to the bugfix (i've seen several code changes which are for a different issue, and alsoquite a few changes where the code was reformatted - adding spaces here and there - these are a big NO in a work environment)
when you create a PR, you have an overview of all the changes - if there are changes which are not relevant to the PR, you should not create a PR, you need to go back to oyur changes and revert those that should not be in the PR
|
|
||
| def add_bloom(*, sender: User, content: str) -> Bloom: | ||
| if len(content) > 280: | ||
| raise ValueError("Bloom content must not exceed 280 character limit.") |
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.
this change should not be in this PR
| const originalText = submitButton.textContent; | ||
| const textarea = form.querySelector("textarea"); | ||
| const content = textarea.value.trim(); | ||
| const charMaxLength = 280; |
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.
these changes should not be in this PR!
| return text.replace( | ||
| /\B#[^#]+/g, | ||
| (match) => `<a href="/hashtag/${match.slice(1)}">${match}</a>` | ||
| /\B#(\w+)/g, |
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.
these changes should not be in this PR!
| destroy(); | ||
|
|
||
| apiService.getBloomsByHashtag(hashtag); | ||
| if (formattedHashtag !== state.currentHashtag) { |
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.
great work introducing a condition on when to check the api!
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.