532 create the frontend for tours #553
Conversation
- Added formik form to set tour appearance. - Added form validation. - Completed preview tab to reflect live changes. - Bug fixes.
- Draggable step component was added. - Added steps functionality.
- Added formik form to set tour appearance. - Added form validation. - Completed preview tab to reflect live changes. - Bug fixes.
- Draggable step component was added. - Added steps functionality.
…m/bluewave-labs/guidefox into 532-create-the-frontend-for-tours
WalkthroughThis pull request introduces several new features for managing product tours. It adds new DnD Kit dependencies for enhanced drag-and-drop functionality and updates the tour routing to enforce authentication. New components such as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TourPage
participant TourLeftContent
participant TourLeftAppearance
participant TourPreview
participant API
User->>TourPage: Open Tour Creation/Edit
TourPage->>TourLeftContent: Render and manage tour steps
User->>TourLeftContent: Reorder or edit steps (drag-and-drop)
TourLeftContent->>TourPage: Update steps state
User->>TourLeftAppearance: Adjust appearance settings
TourLeftAppearance->>TourPage: Validate and update appearance via Formik
User->>TourPreview: Navigate through tour steps for preview
User->>TourPage: Save tour changes
TourPage->>API: Trigger addTour/editTour API call
API-->>TourPage: Return response (success/error)
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (1)
frontend/src/scenes/dashboard/Dashboard.jsx (1)
38-38:⚠️ Potential issueDon't lose yourself: Add 'tour' to metricNames! 📊
The
mapMetricNamefunction supports 'tour' metrics, but 'tour' is missing from themetricNamesarray. Add it to ensure tour statistics are displayed:- const metricNames = ['popup', 'banner', 'link', 'hint'] + const metricNames = ['popup', 'banner', 'link', 'hint', 'tour']
🧹 Nitpick comments (16)
frontend/package.json (1)
7-9: Mom's spaghetti moment: Let's lock these versions down! 🍝While the DnD Kit packages are solid choices for drag-and-drop functionality, using caret versions (
^) could lead to unexpected breaks. Consider pinning the versions:- "@dnd-kit/core": "^6.3.1", - "@dnd-kit/sortable": "^10.0.0", - "@dnd-kit/utilities": "^3.2.2", + "@dnd-kit/core": "6.3.1", + "@dnd-kit/sortable": "10.0.0", + "@dnd-kit/utilities": "3.2.2",frontend/src/scenes/dashboard/Dashboard.jsx (1)
80-84: Knees weak, arms heavy: Tour button needs its own identity! 🎯Using
HintSkeletonfor the tour button might confuse users. Consider creating a dedicatedTourSkeletoncomponent to better represent the tour functionality.// Create a new file: HomePageComponents/Skeletons/TourSkeleton.jsx const TourSkeleton = () => { // Implement tour-specific skeleton UI }; // Update the button configuration { skeletonType: <TourSkeleton />, placeholder: "Create a new tour", onClick: () => navigate("/tour", { state: { autoOpen: true } }), }frontend/src/utils/tourHelper.js (1)
4-4: Yo, the validation schema is straight fire! 🎯The Yup schema implementation is solid with proper validation rules. Just one suggestion to make the color regex case-insensitive for better flexibility.
-const COLOR_REGEX = /^#[0-9A-Fa-f]{6}$/; +const COLOR_REGEX = /^#[0-9A-Fa-f]{6}$/i;Also applies to: 15-41
frontend/src/scenes/tours/TourPreview/TourPreview.module.scss (1)
45-49: Yo, let's make this heading accessible! ♿Add proper ARIA attributes to improve accessibility for screen readers.
h2 { font-size: 1.5rem; font-weight: bold; color: var(--primary-text-color); + /* Add role and aria-level for better accessibility */ + role: heading; + aria-level: 2; }frontend/src/scenes/popup/PopupPageComponents/PopupAppearance/PopupAppearance.jsx (1)
17-19: Yo dawg, we need to handle undefined values!The direct spread of
popupAppearancecould lead to undefined values in the form. Consider adding a default empty object as fallback.initialValues={{ - ...popupAppearance, + ...(popupAppearance || {}), }}frontend/src/components/DraggableTourStep/DraggableTourStep.module.scss (1)
9-9: Consider using CSS variables for these magic numbers!Hardcoded values like
241px,30px, and100pxshould be moved to CSS variables for better maintainability and consistency.:root { + --tour-step-width: 241px; + --tour-button-size: 30px; + --tour-input-width: 100px; } .stepContainer { - width: 241px; + width: var(--tour-step-width); /* ... */ &__button { - height: 30px; - width: 30px; + height: var(--tour-button-size); + width: var(--tour-button-size); } &__customInput { - width: 100px; + width: var(--tour-input-width); } }Also applies to: 19-19, 49-49
frontend/src/templates/GuideTemplate/GuideTemplate.jsx (2)
32-37: Heads up on that state object, fam! 🤔While the optional chaining is solid, consider making the empty state object more explicit about what properties are being cleared:
- if (location.state?.autoOpen) navigate('/', { state: {} }); + if (location.state?.autoOpen) navigate('/', { state: { autoOpen: false } });
75-99: Let's level up that switch component! 💪The switch implementation could use some accessibility love:
<Switch id="switch" name="isActive" value={switchValue} onChange={onSwitchChange} + aria-label="Toggle active state" /> <label + htmlFor="switch" style={{ fontSize: 'var(--font-regular)', alignSelf: 'center', }} >Also, consider moving those inline styles to your CSS module for better maintainability.
frontend/src/components/DropdownList/DropdownList.jsx (2)
14-14: Watch out for style collisions, dawg!⚠️ While spreading styles is flexible, it could override important base styles. Consider using a whitelist approach:
- sx={{ marginTop: 1, ...styles }} + sx={{ marginTop: 1, ...(styles?.custom || {}) }}Also applies to: 46-46
18-26: Let's make this function zoom! 🏃♂️The case-insensitive comparison could be optimized by converting to lowercase once:
const getInitialSelectedAction = () => { if (selectedActionString) { + const searchString = selectedActionString.toLowerCase(); const index = actions.findIndex( - (action) => action.toLowerCase() === selectedActionString.toLowerCase() + (action) => action.toLowerCase() === searchString ); return index !== -1 ? actions[index] : actions[0] || ''; } return actions[selectedActionIndex] || ''; };frontend/src/components/DraggableTourStep/DraggableTourStep.jsx (1)
16-29: Let's make this drag-n-drop accessible! ♿Add ARIA attributes to improve accessibility:
const style = { transform: CSS.Transform.toString(transform), transition, opacity: isDragging ? 0.5 : 1, + cursor: isDragging ? 'grabbing' : 'grab', };frontend/src/scenes/tours/CreateTourPage.jsx (1)
17-26: Consider using a more descriptive initial step content.The default step content appears to be hardcoded with GuideFox-specific content. This might not be suitable for all use cases.
const [stepsData, setStepsData] = useState([ { id: 0, stepName: 'Step 1', - header: 'Welcome to GuideFox', - content: - 'Serve your users and increase product adoption with hints, popups, banners, and helper links. \n\nEarn an extra 30% if you purchase an annual plan with us.', + header: 'Welcome', + content: 'Add your tour content here...', targetElement: '', }, ]);frontend/src/scenes/tours/TourPageComponents/TourleftAppearance/TourLeftAppearance.jsx (1)
103-122: Add URL validation feedback.Consider adding real-time URL validation feedback to improve user experience.
<CustomTextField TextFieldWidth="206px" name="url" value={values.url} + helperText="Enter a valid URL starting with http:// or https://" + error={Boolean(touched.url && errors.url)} onChange={(e) => { handleChange({ target: { name: 'url', value: e.target.value }, }); setTourPopupAppearance((prev) => ({ ...prev, url: e.target.value, })); }} onBlur={(e) => { handleBlur(e); validateField('url'); }} />frontend/src/services/tourServices.js (1)
1-60: Bruh, we need some retry logic for those flaky network calls!Consider implementing a retry mechanism for failed API requests using exponential backoff.
const retryWithBackoff = async (fn, maxRetries = 3) => { for (let i = 0; i < maxRetries; i++) { try { return await fn(); } catch (error) { if (i === maxRetries - 1) throw error; await new Promise(resolve => setTimeout(resolve, Math.pow(2, i) * 1000)); } } };frontend/src/scenes/tours/TourPageComponents/TourLeftContent/TourLeftContent.jsx (2)
71-81: That delete handler needs some love! 💔The delete handler should update step names to maintain sequential order.
const deleteHandler = (identity) => { if (stepsData.length === 1) return; const updatedSteps = stepsData.filter(({ id }) => id !== identity); + const renamedSteps = updatedSteps.map((step, index) => ({ + ...step, + stepName: `Step ${index + 1}`, + })); if (identity === currentStep.id) { - setCurrentStep(updatedSteps[0]); + setCurrentStep(renamedSteps[0]); } - setStepsData(updatedSteps); + setStepsData(renamedSteps); };
95-153: Performance optimization time! 🚀Consider memoizing the handlers and using
useCallbackto prevent unnecessary re-renders.const memoizedHandlers = { onDragStart: useCallback(({ active }) => setActiveDragId(active.id), []), onDragEnd: useCallback(handleDragEnd, [setStepsData, setActiveDragId]), onSelect: useCallback(selectHandler, [stepsData, setCurrentStep]), onDelete: useCallback(deleteHandler, [stepsData, currentStep, setCurrentStep, setStepsData]) };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/public/no-background.jpgis excluded by!**/*.jpg
📒 Files selected for processing (30)
frontend/package.json(1 hunks)frontend/src/App.jsx(2 hunks)frontend/src/assets/icons/utilityIcons.jsx(3 hunks)frontend/src/components/DraggableTourStep/DraggableTourStep.jsx(1 hunks)frontend/src/components/DraggableTourStep/DraggableTourStep.module.scss(1 hunks)frontend/src/components/DropdownList/DropdownList.jsx(3 hunks)frontend/src/components/Links/ColorInput/ColorInput.module.scss(1 hunks)frontend/src/components/Links/ColorInput/index.jsx(1 hunks)frontend/src/components/RichTextEditor/RichTextEditor.jsx(2 hunks)frontend/src/components/TextFieldComponents/CustomTextField/CustomTextField.jsx(3 hunks)frontend/src/data/guideMainPageData.js(1 hunks)frontend/src/scenes/dashboard/Dashboard.jsx(1 hunks)frontend/src/scenes/popup/PopupPageComponents/PopupAppearance/PopupAppearance.jsx(1 hunks)frontend/src/scenes/tours/CreateTourPage.jsx(1 hunks)frontend/src/scenes/tours/CreateToursPopup/CreateToursPopup.jsx(0 hunks)frontend/src/scenes/tours/CreateToursPopup/CreateToursPopup.module.scss(0 hunks)frontend/src/scenes/tours/ProductTour.jsx(0 hunks)frontend/src/scenes/tours/ProductTourStyles.css(0 hunks)frontend/src/scenes/tours/TourDefaultPage.jsx(1 hunks)frontend/src/scenes/tours/TourPageComponents/TourLeftContent/TourLeftContent.jsx(1 hunks)frontend/src/scenes/tours/TourPageComponents/TourLeftContent/TourLeftContent.module.scss(1 hunks)frontend/src/scenes/tours/TourPageComponents/TourleftAppearance/TourLeftAppearance.jsx(1 hunks)frontend/src/scenes/tours/TourPageComponents/TourleftAppearance/TourLeftAppearance.module.scss(1 hunks)frontend/src/scenes/tours/TourPreview/TourPreview.jsx(1 hunks)frontend/src/scenes/tours/TourPreview/TourPreview.module.scss(1 hunks)frontend/src/scenes/tours/ToursDefaultPage.jsx(0 hunks)frontend/src/services/tourServices.js(1 hunks)frontend/src/templates/GuideMainPageTemplate/GuideMainPageTemplate.css(1 hunks)frontend/src/templates/GuideTemplate/GuideTemplate.jsx(4 hunks)frontend/src/utils/tourHelper.js(1 hunks)
💤 Files with no reviewable changes (5)
- frontend/src/scenes/tours/ToursDefaultPage.jsx
- frontend/src/scenes/tours/CreateToursPopup/CreateToursPopup.module.scss
- frontend/src/scenes/tours/ProductTourStyles.css
- frontend/src/scenes/tours/ProductTour.jsx
- frontend/src/scenes/tours/CreateToursPopup/CreateToursPopup.jsx
✅ Files skipped from review due to trivial changes (2)
- frontend/src/scenes/tours/TourPageComponents/TourleftAppearance/TourLeftAppearance.module.scss
- frontend/src/scenes/tours/TourPageComponents/TourLeftContent/TourLeftContent.module.scss
🔇 Additional comments (7)
frontend/src/templates/GuideMainPageTemplate/GuideMainPageTemplate.css (1)
24-26: Yo, this flex layout is straight fire! 🔥The addition of
flex: 1allows the container to grow and fill available space, maintaining a clean layout structure.frontend/src/components/Links/ColorInput/ColorInput.module.scss (1)
75-82: These styles are cleaner than my sweater! ✨Excellent use of CSS variables and BEM methodology for error state styling. The implementation maintains consistency with the design system.
frontend/src/components/TextFieldComponents/CustomTextField/CustomTextField.jsx (1)
15-15: Yo, these changes are fire! 🔥The addition of
labelTextStylesprop with proper PropTypes definition and implementation looks clean. The spread operator usage in thesxprop is on point!Also applies to: 46-46, 124-124
frontend/src/components/RichTextEditor/RichTextEditor.jsx (1)
46-49: Yo, why we commenting out this cleanup code?Removing the cleanup logic in
onDestroycould lead to stale state when the editor is destroyed and recreated. Consider keeping the cleanup or document why it's not needed.frontend/src/templates/GuideTemplate/GuideTemplate.jsx (1)
12-25: Yo, these prop changes are fire! 🔥The new props with their default values are well-structured and type-safe. Nice job handling the defaults to prevent undefined behavior.
frontend/src/App.jsx (1)
43-43: Secure route game strong! 🔒Adding authentication protection with the Private component wrapper is a solid security move. The naming convention update to TourDefaultPage is also more consistent with the singular form used in other routes.
frontend/src/data/guideMainPageData.js (1)
38-44: LGTM! The tour activity info is well-structured.The content is clear, concise, and follows the same pattern as other activity types. The description effectively communicates the purpose and value of product tours.
| getItems={getTours} | ||
| deleteItem={deleteTour} | ||
| setIsEdit={setIsEdit} | ||
| setItemId={setItemId} | ||
| itemType={ACTIVITY_TYPES_INFO.TOURS} | ||
| itemTypeInfo={ACTIVITY_TYPES_INFO.TOURS} | ||
| getItemDetails={getTourDetails} | ||
| itemsUpdated={itemsUpdated} | ||
| getItemById={getTourById} | ||
| duplicateItem={addTour} | ||
| /> |
There was a problem hiding this comment.
Yo, we need some error handling in here! 🚨
The service calls (getTours, deleteTour, getTourById, addTour) need proper error handling to manage API failures gracefully.
<DefaultPageTemplate
getItems={getTours}
- deleteItem={deleteTour}
+ deleteItem={async (id) => {
+ try {
+ await deleteTour(id);
+ } catch (error) {
+ console.error('Failed to delete tour:', error);
+ throw new Error('Failed to delete tour. Please try again.');
+ }
+ }}
setIsEdit={setIsEdit}
setItemId={setItemId}
itemType={ACTIVITY_TYPES_INFO.TOURS}
itemTypeInfo={ACTIVITY_TYPES_INFO.TOURS}
getItemDetails={getTourDetails}
itemsUpdated={itemsUpdated}
- getItemById={getTourById}
+ getItemById={async (id) => {
+ try {
+ return await getTourById(id);
+ } catch (error) {
+ console.error('Failed to fetch tour:', error);
+ throw new Error('Failed to fetch tour details. Please try again.');
+ }
+ }}
- duplicateItem={addTour}
+ duplicateItem={async (data) => {
+ try {
+ return await addTour(data);
+ } catch (error) {
+ console.error('Failed to duplicate tour:', error);
+ throw new Error('Failed to duplicate tour. Please try again.');
+ }
+ }}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getItems={getTours} | |
| deleteItem={deleteTour} | |
| setIsEdit={setIsEdit} | |
| setItemId={setItemId} | |
| itemType={ACTIVITY_TYPES_INFO.TOURS} | |
| itemTypeInfo={ACTIVITY_TYPES_INFO.TOURS} | |
| getItemDetails={getTourDetails} | |
| itemsUpdated={itemsUpdated} | |
| getItemById={getTourById} | |
| duplicateItem={addTour} | |
| /> | |
| getItems={getTours} | |
| deleteItem={async (id) => { | |
| try { | |
| await deleteTour(id); | |
| } catch (error) { | |
| console.error('Failed to delete tour:', error); | |
| throw new Error('Failed to delete tour. Please try again.'); | |
| } | |
| }} | |
| setIsEdit={setIsEdit} | |
| setItemId={setItemId} | |
| itemType={ACTIVITY_TYPES_INFO.TOURS} | |
| itemTypeInfo={ACTIVITY_TYPES_INFO.TOURS} | |
| getItemDetails={getTourDetails} | |
| itemsUpdated={itemsUpdated} | |
| getItemById={async (id) => { | |
| try { | |
| return await getTourById(id); | |
| } catch (error) { | |
| console.error('Failed to fetch tour:', error); | |
| throw new Error('Failed to fetch tour details. Please try again.'); | |
| } | |
| }} | |
| duplicateItem={async (data) => { | |
| try { | |
| return await addTour(data); | |
| } catch (error) { | |
| console.error('Failed to duplicate tour:', error); | |
| throw new Error('Failed to duplicate tour. Please try again.'); | |
| } | |
| }} | |
| /> |
| @@ -83,7 +83,7 @@ const RichTextEditor = ({ | |||
| setMode={setMode} | |||
| sx={{ | |||
| position: "absolute", | |||
| bottom: "35px", | |||
| bottom: "px", | |||
There was a problem hiding this comment.
Fix this broken style value, fam!
The bottom style value is invalid. It's missing a number before "px".
- bottom: "px",
+ bottom: "35px",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bottom: "px", | |
| bottom: "35px", |
| @@ -35,3 +35,15 @@ export const EditIcon = () => ( | |||
| /> | |||
| </IconWrapper> | |||
| ); | |||
|
|
|||
| export const Hamburger = ({ styles }) => ( | |||
| <IconWrapper style={styles} role="img" aria-label="Delete item"> | |||
There was a problem hiding this comment.
Fix that accessibility label, it's not right!
The aria-label "Delete item" is incorrect for a hamburger menu icon. It should describe its actual purpose.
- <IconWrapper style={styles} role="img" aria-label="Delete item">
+ <IconWrapper style={styles} role="img" aria-label="Drag handle">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <IconWrapper style={styles} role="img" aria-label="Delete item"> | |
| <IconWrapper style={styles} role="img" aria-label="Drag handle"> |
| &:blur { | ||
| border: 1px solid transparent; | ||
| background-color: var(--header-background); | ||
| } |
There was a problem hiding this comment.
Yo, that :blur ain't a real pseudo-class!
The :blur pseudo-class doesn't exist in CSS. You probably meant to use the blur event with JavaScript or the :focus-within pseudo-class.
- &:blur {
+ &:not(:focus) {
border: 1px solid transparent;
background-color: var(--header-background);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| &:blur { | |
| border: 1px solid transparent; | |
| background-color: var(--header-background); | |
| } | |
| &:not(:focus) { | |
| border: 1px solid transparent; | |
| background-color: var(--header-background); | |
| } |
| console.error('Add Tour error:', error.response.data.errors); | ||
| throw error; |
There was a problem hiding this comment.
Yo, we're leaking sensitive info in those error logs! 🚨
The error logging is exposing potentially sensitive information from the API responses. Consider implementing a sanitized error logging mechanism.
- console.error('Add Tour error:', error.response.data.errors);
+ console.error('Add Tour error:', 'Failed to add tour. Please try again.');Also applies to: 18-19, 28-32, 41-45, 54-58
| export const addTour = async (tourData) => { | ||
| try { | ||
| const response = await apiClient.post('/tour/add_tour', tourData); | ||
| return response.data; | ||
| } catch (error) { | ||
| console.error('Add Tour error:', error.response.data.errors); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Yo dawg, we need to validate that tour data before sending it to the server!
The addTour function should validate the input data before making the API call to prevent invalid data from reaching the server.
export const addTour = async (tourData) => {
+ if (!tourData || typeof tourData !== 'object') {
+ throw new Error('Invalid tour data provided');
+ }
try {
const response = await apiClient.post('/tour/add_tour', tourData);
return response.data;
} catch (error) {
- console.error('Add Tour error:', error.response.data.errors);
+ console.error('Add Tour error:', error.response?.data?.errors || error.message);
throw error;
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const addTour = async (tourData) => { | |
| try { | |
| const response = await apiClient.post('/tour/add_tour', tourData); | |
| return response.data; | |
| } catch (error) { | |
| console.error('Add Tour error:', error.response.data.errors); | |
| throw error; | |
| } | |
| }; | |
| export const addTour = async (tourData) => { | |
| if (!tourData || typeof tourData !== 'object') { | |
| throw new Error('Invalid tour data provided'); | |
| } | |
| try { | |
| const response = await apiClient.post('/tour/add_tour', tourData); | |
| return response.data; | |
| } catch (error) { | |
| console.error('Add Tour error:', error.response?.data?.errors || error.message); | |
| throw error; | |
| } | |
| }; |
| const handleChange = (e) => { | ||
| const val = e.target.value; | ||
|
|
||
| setValue(val); | ||
| onChange(val); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Yo, we need to validate those color values! 🎨
Add color validation to ensure only valid hex colors are accepted.
const handleChange = (e) => {
const val = e.target.value;
+ const isValidHex = /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/;
+ if (!isValidHex.test(val)) {
+ return;
+ }
setValue(val);
onChange(val);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleChange = (e) => { | |
| const val = e.target.value; | |
| setValue(val); | |
| onChange(val); | |
| }; | |
| const handleChange = (e) => { | |
| const val = e.target.value; | |
| const isValidHex = /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/; | |
| if (!isValidHex.test(val)) { | |
| return; | |
| } | |
| setValue(val); | |
| onChange(val); | |
| }; |
| <div className={styles.appearance__circle}> | ||
| <input | ||
| type="color" | ||
| id={id} | ||
| value={field.value} | ||
| onChange={handleChange} | ||
| style={{ zIndex: 10, cursor: 'pointer', opacity: 0 }} | ||
| onBlur={() => setTouched(true)} | ||
| /> | ||
| {error && touched ? ( | ||
| <img | ||
| src="/no-background.jpg" | ||
| alt="No color selected" | ||
| className={styles['appearance__circle--mask']} | ||
| /> | ||
| ) : ( | ||
| <span | ||
| className={styles['appearance__circle--mask']} | ||
| style={{ backgroundColor: field.value }} | ||
| /> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Accessibility game weak! Let's boost those ARIA attributes! 🔥
The color picker needs better accessibility support.
<input
type="color"
id={id}
value={field.value}
onChange={handleChange}
style={{ zIndex: 10, cursor: 'pointer', opacity: 0 }}
onBlur={() => setTouched(true)}
+ aria-label={`Choose ${title} color`}
+ role="button"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className={styles.appearance__circle}> | |
| <input | |
| type="color" | |
| id={id} | |
| value={field.value} | |
| onChange={handleChange} | |
| style={{ zIndex: 10, cursor: 'pointer', opacity: 0 }} | |
| onBlur={() => setTouched(true)} | |
| /> | |
| {error && touched ? ( | |
| <img | |
| src="/no-background.jpg" | |
| alt="No color selected" | |
| className={styles['appearance__circle--mask']} | |
| /> | |
| ) : ( | |
| <span | |
| className={styles['appearance__circle--mask']} | |
| style={{ backgroundColor: field.value }} | |
| /> | |
| )} | |
| </div> | |
| <div className={styles.appearance__circle}> | |
| <input | |
| type="color" | |
| id={id} | |
| value={field.value} | |
| onChange={handleChange} | |
| style={{ zIndex: 10, cursor: 'pointer', opacity: 0 }} | |
| onBlur={() => setTouched(true)} | |
| aria-label={`Choose ${title} color`} | |
| role="button" | |
| /> | |
| {error && touched ? ( | |
| <img | |
| src="/no-background.jpg" | |
| alt="No color selected" | |
| className={styles['appearance__circle--mask']} | |
| /> | |
| ) : ( | |
| <span | |
| className={styles['appearance__circle--mask']} | |
| style={{ backgroundColor: field.value }} | |
| /> | |
| )} | |
| </div> |
| const addNewStepHandler = () => { | ||
| setStepsData((prev) => [ | ||
| ...prev, | ||
| { | ||
| ...defaultStep, | ||
| id: Date.now(), | ||
| stepName: `Step ${prev.length + 1}`, | ||
| }, | ||
| ]); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bruh, that Date.now() ID generation is sketchy! 🤔
Using timestamps as IDs could lead to collisions if steps are created simultaneously. Consider using a UUID instead.
+ import { v4 as uuidv4 } from 'uuid';
const addNewStepHandler = () => {
setStepsData((prev) => [
...prev,
{
...defaultStep,
- id: Date.now(),
+ id: uuidv4(),
stepName: `Step ${prev.length + 1}`,
},
]);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const addNewStepHandler = () => { | |
| setStepsData((prev) => [ | |
| ...prev, | |
| { | |
| ...defaultStep, | |
| id: Date.now(), | |
| stepName: `Step ${prev.length + 1}`, | |
| }, | |
| ]); | |
| }; | |
| import { v4 as uuidv4 } from 'uuid'; | |
| const addNewStepHandler = () => { | |
| setStepsData((prev) => [ | |
| ...prev, | |
| { | |
| ...defaultStep, | |
| id: uuidv4(), | |
| stepName: `Step ${prev.length + 1}`, | |
| }, | |
| ]); | |
| }; |
Describe your changes
Created frontend for tour
Issue number
#532
Please ensure all items are checked off before requesting a review:
Preview