-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fixes Using Chips in the "add Tags" section #9103 #9508
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.
PRs not related to profile, should not include any data file. Also, you're not allowed to remove someone else json file.
Please revert the changes from data/alevel7.json
This reverts commit 8ba32c0.
I reverted that commit and pushed the new changes. Sorry for accidently deleting someone's json file. |
components/tag/TagsInput.js
Outdated
import XMarkIcon from "@heroicons/react/20/solid/XMarkIcon"; | ||
import Input from "../form/Input"; | ||
|
||
export default function TagsInput({ tags, onTagAdd, onTagRemove }) { | ||
const inputRef = useRef(null); | ||
export default function TagsInput({ tags, onTagAdd, onTagRemove, tagInputRef }) { |
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.
If this name was kept the same there would be less changes in the file. I think `inputRef is still descriptive in the scope of this function
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.
so should I the tagInputref
to inputRef
?
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.
Yes please 👍
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.
I clicked on manage events, then went to edit an event. You can search for where the |
in my local setup there is no option to add tags on events and VS Code is showing that |
see screen recording below to reproduce the error Onboarding.Dashboard.-.Google.Chrome.mp4 |
@@ -57,6 +57,7 @@ export default function ManageEvent({ BASE_URL, event }) { | |||
const [endDate, setEndDate] = useState(""); | |||
const [speakingTopic, setspeakingTopic] = useState(event.speakingTopic || ""); | |||
const [tags, setTags] = useState(event.tags || []); | |||
const inputRef = useRef(null); |
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.
I'm not sure why this variable named inputRef
. I think naming the variable simply inputRef
might not be very descriptive, because I see in the scope of this function has multiple input fields.
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.
Yep 👍 Inside the tag component it can be inputRef
but outside it should be tagInputRef
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.
Yeah that makes 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.
sorry for the late reply, I made the changes as you requested and pushed the changes.
pages/account/manage/profile.js
Outdated
@@ -76,6 +76,8 @@ export default function Profile({ BASE_URL, profile, fileExists }) { | |||
}; | |||
}); | |||
|
|||
const inputRef = useRef(null); |
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.
Same here
@@ -105,6 +106,10 @@ export default function ManageEvent({ BASE_URL, event }) { | |||
const handleSubmit = async (e) => { | |||
e.preventDefault(); | |||
|
|||
if (document.activeElement === tagInputRef.current) { | |||
console.log('testing'); |
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.
Please remove debug code
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.
Sorry I forgot to remove it before now I've removed it and pushed the changes. Thanks for addressing 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.
Looks good to me👍
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.
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.
Sorry I was meant to approve
Fixes Issue
Fixes #9103
Changes proposed
Added "enter" functionality to the "Tags" input, now we can add new tags by pressing "enter".
Screenshots
2023-10-15.14-57-01.mp4