Vps 058/speechtext theme #97
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arnard76
commented
May 23, 2023
There was a problem hiding this comment.
copied from TextPropertiesComponent.js but removed the border property because it doesn't apply to speech textboxes. More info in the relevant commit.
1 task
NOTE: theme is the same TEXT component but that will be changed in: Choose a speech bubble theme to add to simulator #58
Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json. fix: removing package-lock.json
for the arrow/pointing thingy in the speechbox 💬
put into component SpeechboxArrow.js allowing absolute imports to frontend 🤯🤯😁👇 SpeechboxArrow.js will be in src/components folder but will be used from SpeechTextComponent.js (will be added in next commit 👀) SpeechTextComponent.js & SpeechboxArrow.js are very far and using relative imports => PAIN. followed this guide to setup absolute imports: https://plainenglish.io/blog/why-and-how-to-use-absolute-imports-in-react
* based of text component ( + a pointer to the speaker) updating component resolver & styles for speechtext component
…n text component ... because a textbox needs to have a border. Without a border it looks the same as textbox 🤦
div element in the new SpeechTextComponent was the first element to trigger the keypress event because it was focussable. This meant that the event.target.tagName was "DIV" & the deleteElement() function isn't called 🥲 to solve this issue 🛠️🛠️: capture keydown event (delete) on body so event is triggered (& captured) by the body element.
This reverts commit 51fcf46.
because tabIndex + textbox role results in div triggering a keypress element. On the Canvas.js compenent, this results in deleteElement not being called. fixed by removing tabIndex and role from speechbox component. Possible TODO: All other components aren't accessable by a11y guidelines (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA) but the current way of deleting elements requires components to be non-focussable.
`flex:1` so textbox fils the entire height of the component
…e speechbox arrow 🙂
including a snapshot (that includes all components inside SpeechTextComponent like Triangle)
put into component SpeechboxArrow.js allowing absolute imports to frontend 🤯🤯😁👇 SpeechboxArrow.js will be in src/components folder but will be used from SpeechTextComponent.js (will be added in next commit 👀) SpeechTextComponent.js & SpeechboxArrow.js are very far and using relative imports => PAIN. followed this guide to setup absolute imports: https://plainenglish.io/blog/why-and-how-to-use-absolute-imports-in-react
* based of text component ( + a pointer to the speaker) updating component resolver & styles for speechtext component
div element in the new SpeechTextComponent was the first element to trigger the keypress event because it was focussable. This meant that the event.target.tagName was "DIV" & the deleteElement() function isn't called 🥲 to solve this issue 🛠️🛠️: capture keydown event (delete) on body so event is triggered (& captured) by the body element.
This reverts commit 51fcf46.
because tabIndex + textbox role results in div triggering a keypress element. On the Canvas.js compenent, this results in deleteElement not being called. fixed by removing tabIndex and role from speechbox component. Possible TODO: All other components aren't accessable by a11y guidelines (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA) but the current way of deleting elements requires components to be non-focussable.
`flex:1` so textbox fils the entire height of the component
…e speechbox arrow 🙂
put into component SpeechboxArrow.js allowing absolute imports to frontend 🤯🤯😁👇 SpeechboxArrow.js will be in src/components folder but will be used from SpeechTextComponent.js (will be added in next commit 👀) SpeechTextComponent.js & SpeechboxArrow.js are very far and using relative imports => PAIN. followed this guide to setup absolute imports: https://plainenglish.io/blog/why-and-how-to-use-absolute-imports-in-react
* based of text component ( + a pointer to the speaker) updating component resolver & styles for speechtext component
div element in the new SpeechTextComponent was the first element to trigger the keypress event because it was focussable. This meant that the event.target.tagName was "DIV" & the deleteElement() function isn't called 🥲 to solve this issue 🛠️🛠️: capture keydown event (delete) on body so event is triggered (& captured) by the body element.
This reverts commit 51fcf46.
because tabIndex + textbox role results in div triggering a keypress element. On the Canvas.js compenent, this results in deleteElement not being called. fixed by removing tabIndex and role from speechbox component. Possible TODO: All other components aren't accessable by a11y guidelines (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA) but the current way of deleting elements requires components to be non-focussable.
`flex:1` so textbox fils the entire height of the component
…e speechbox arrow 🙂
put into component SpeechboxArrow.js allowing absolute imports to frontend 🤯🤯😁👇 SpeechboxArrow.js will be in src/components folder but will be used from SpeechTextComponent.js (will be added in next commit 👀) SpeechTextComponent.js & SpeechboxArrow.js are very far and using relative imports => PAIN. followed this guide to setup absolute imports: https://plainenglish.io/blog/why-and-how-to-use-absolute-imports-in-react
* based of text component ( + a pointer to the speaker) updating component resolver & styles for speechtext component
div element in the new SpeechTextComponent was the first element to trigger the keypress event because it was focussable. This meant that the event.target.tagName was "DIV" & the deleteElement() function isn't called 🥲 to solve this issue 🛠️🛠️: capture keydown event (delete) on body so event is triggered (& captured) by the body element.
This reverts commit 51fcf46.
because tabIndex + textbox role results in div triggering a keypress element. On the Canvas.js compenent, this results in deleteElement not being called. fixed by removing tabIndex and role from speechbox component. Possible TODO: All other components aren't accessable by a11y guidelines (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA) but the current way of deleting elements requires components to be non-focussable.
`flex:1` so textbox fils the entire height of the component
…e speechbox arrow 🙂
arnard76
force-pushed
the
VPS-058/speechtext-theme
branch
from
22710e7
to
a3cf19f
Compare
index.js:1 Warning: Cannot update a component (`SpeechTextComponent`) while rendering a different component (`_c`). *NOTE `_c` is the SpeechboxArrow component fix 🛠️: only set the `arrowWidth` state after component has been created using the `useEffect` hook
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Describe the issue
A clear and concise description of what the issue is.
Described by #58
Describe the solution
A clear and concise description of what the solution is.
Creating a new component: SPEECHTEXT
new component looks different to the TEXT component because there is an arrow pointing to a speaker 🙂◀️
Speechbox theme:
Warning fixed: when using yarn, avoid using npm => Removed
package-lock.jsonDev feature added🧑💻: can use absolute imports in code
e.g.
import Triangle from "components/Triangle";This makes it simpler to import components that are located far away from component doing the importing, but close to the
srcfolder.NOTE: relative imports like this still work:
import SpeechTextPropertiesComponent from "./ComponentProperties/SpeechTextPropertiesComponent";updated snapshots for tests and fixed prior css formatting issues
Risk
Potential risks that this PR might brings
I don't know of any yet.
Definition of Done
Reviewed By
Who reviewed your PR - for commit history once merged