feat: Restrict attemptType based on style selected for ticks #1260

glassbead0 · 2025-01-24T23:59:27Z

name: Pull request
about: Create a pull request
title: 'Restrict tick Attempt Type'
labels: ''
assignees: ''

What type of PR is this?(check all applicable)

Description

Related Issues

Issue #411

What this PR achieves

add the "Tick the Route" button to the newer climbs page (it was previously only on the old page)
dynamically populate available attemptType based on chosen style
display attemptType in TickCard when viewing listed ticks
add subtle info icon with link to Wikipedia's Glossary of Climbing Terms
tinker with z values because the breadcrumb nav bar was overlapping the tick dialog
increase notes size slightly to accomodate for longer list of attempt styles in the dropdown.

Screenshots, recordings

Notes

this is closely tied to OpenBeta/openbeta-graphql#444. and is essentially the front-end version of the same restrictions.

One thing i could use help with:
Style and attemptType can be empty, but right now thats getting sent as a string with a space. I'd like them to be nullable, but was running into issues with that.

glassbead0 · 2025-01-26T01:31:02Z

src/components/users/TickCard.tsx

@@ -37,7 +38,10 @@ export default function TickCard ({ tickId, ticks, setTicks, dateClimbed, notes,
        <p className='text-sm text-gray-500 mb-0'>{notes}</p>
      </div>
      <div className='flex flex-row'>
-        <p className='text-sm text-gray-500 pr-5'>{style}</p>
+        <div className='flex flex-col'>


This shows the attemptType on the tick card:

glassbead0 · 2025-01-26T01:32:56Z

src/pages/climbs/[id].tsx

@@ -440,7 +440,7 @@ const ClimbData: React.FC<{

        {!editMode && (
          <div className='mt-8'>
-            <TickButton climbId={climbId} name={name} grade={gradesObj.toString()} />
+            <TickButton climbId={climbId} name={name} grade={gradesObj.toString()} climbType={{}} />


Maybe I should just remove this tick buttons, since this is the "old" page....

you can leave the button there until we delete the page

glassbead0 · 2025-01-26T01:33:28Z

src/components/users/TicksModal.tsx

@@ -12,7 +12,7 @@ const TicksModal: React.FC<any> = ({ open, setOpen, setTicks, setOpenForm, climb
  }
  return (
    <Transition.Root show={open} as={Fragment}>
-      <Dialog as='div' className='relative z-10' initialFocus={cancelButtonRef} onClose={setOpen}>


These z-values were needed so the breadcrumb nav bar didn't obscure the Tick modal.

glassbead0 · 2025-01-26T01:34:24Z

src/components/users/TickForm.tsx

+                    <label htmlFor='attemptType' className='block text-sm font-medium text-gray-700'>
+                      Attempt Type
+                    </label>
+                    <a href={climbingGlossaryLink} target='_blank' rel='noopener noreferrer' className='ml-2 text-gray-500 hover:text-gray-700' title='climbing terminology'>


The little info button is a link to Climbing Glossary.

We have a Tooltip component you can use.

Code https://github.com/OpenBeta/open-tacos/blob/develop/src/components/edit/form/AreaDesignationRadioGroup.tsx#L39-L41

vnugent · 2025-01-26T04:55:14Z

Does this PR supersede #1259? As commented in #1259 the backend doesn't like empty string for attemptType. Maybe use 'N/A'?

vnugent · 2025-01-26T05:04:20Z

src/components/users/TickForm.tsx

@@ -7,6 +7,7 @@ import { graphqlClient } from '../../js/graphql/Client'
 import { MUTATION_ADD_TICK } from '../../js/graphql/gql/fragments'
 import ComboBox from '../ui/ComboBox'
 import * as Yup from 'yup'
+import { InformationCircleIcon } from '@heroicons/react/20/solid'


We're deprecating @heroicons. Please use https://phosphoricons.com/

Example: https://github.com/OpenBeta/open-tacos/blob/develop/src/components/media/PhotoUploadButtons.tsx#L1

vnugent · 2025-01-26T05:56:29Z

src/components/users/TickForm.tsx

+                    <label htmlFor='attemptType' className='block text-sm font-medium text-gray-700'>
+                      Attempt Type
+                    </label>
+                    <a href={climbingGlossaryLink} target='_blank' rel='noopener noreferrer' className='ml-2 text-gray-500 hover:text-gray-700' title='climbing terminology'>


We have a Tooltip component you can use.

Code https://github.com/OpenBeta/open-tacos/blob/develop/src/components/edit/form/AreaDesignationRadioGroup.tsx#L39-L41

vnugent · 2025-01-26T05:57:14Z

src/pages/climbs/[id].tsx

@@ -440,7 +440,7 @@ const ClimbData: React.FC<{

        {!editMode && (
          <div className='mt-8'>
-            <TickButton climbId={climbId} name={name} grade={gradesObj.toString()} />
+            <TickButton climbId={climbId} name={name} grade={gradesObj.toString()} climbType={{}} />


you can leave the button there until we delete the page

glassbead0 · 2025-01-26T18:42:39Z

Does this PR supersede #1259? As commented in #1259 the backend doesn't like empty string for attemptType. Maybe use 'N/A'?

It does not supersede it. #1259 deals with MP imports, and this one is all for the OB interface for adding ticks. I can certainly combine them, as they are related. I tend to like to combine related work into single PRs, but I also appreciate the value of separating things more granularly.

glassbead0 · 2025-01-30T23:17:37Z

src/components/users/TickButton.tsx

@@ -1,3 +1,4 @@
+'use client'


I was getting errors locally telling me to add this here. "You're importing a component that needs useState. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default. Learn more: https://nextjs.org/docs/getting-started/react-essentials"

I don't know if thats a local-only issue for me though.

We do want to address that error. Generally, we want to have as many server-side components as possible and add "use client" directive as escape hatch. Maybe don't introduce "hover" behavior as it only works on desktop.

I'm happy to remove the hover functionality, but I don't think thats related to the 'use client' error.

Prior to my code changes, the TickButton relied on useState to determine if a route has been ticked by a user, so it knows whether to display Tick This Route or View Ticks on the button.

I'm not sure how to rewrite that functionality without useState, but i'm not at all an expert on Next.js.

I do see several other components that 'use client', so i'm not sure what the best solution is here.

glassbead0 · 2025-01-30T23:21:19Z

src/components/users/TickForm.tsx

+  if (aidable) { styles.push(...allStyles.filter(style => ['Aid'].includes(style.name))) }
+  if (soloable) { styles.push(...allStyles.filter(style => ['Solo'].includes(style.name))) }
+  if (styles.length === 0) { styles = [...allStyles] } // If a climb doesn't have a type, anything goes
+  styles.push({ id: 10, name: '\u00A0' })


FYI the '\u00A0' is the html encoded space character. this is to get the dropdown to show a blank option (and look nice). Down below, It handles this case and sends undefined to the backend, which gets handles correctly in this pr OpenBeta/openbeta-graphql#444

vnugent · 2025-02-05T01:07:01Z

src/components/ui/Tooltip.tsx


 interface Props {
  content: string | ReactNode
  enabled?: boolean
  children: JSX.Element | JSX.Element [] | null
  className?: string
+  trigger?: 'click' | 'hover'


I believe we default to click because hover isn't usable on web mobile

i did set default to click below on line 16, but regardless you're right, hover won't work on mobile. that said, my change here doesn't change the behavior of any other tooltip, only this specific one (which i wanted as a hover, because when you click it, it opens a link in a new tab).

I know this is an unnecessary tooltip at all though, and 'm totally happy to remove it if you think thats better.

Let's not introduce a new behavior for Tooltip, and keep it consistent with existing UX (onClick show a tip, include a clickable external link if needed)

vnugent · 2025-02-05T01:09:48Z

src/components/users/TickButton.tsx

@@ -1,3 +1,4 @@
+'use client'


We do want to address that error. Generally, we want to have as many server-side components as possible and add "use client" directive as escape hatch. Maybe don't introduce "hover" behavior as it only works on desktop.

* add the "Tick the Route" button to the newer climbs page (it was previously only on the old page) * dynamically populate available attemptType based on chosen style * display attemptType in TickCard when viewing listed ticks * add subtle info icon with link to Wikipedia's Glossary of Climbing Terms * tinker with z values because the breadcrumb nav bar was overlapping the tick dialog * increase notes size slightly to accomodate for longer list of attempt styles in the dropdown.

* use phosphor icons * use existing Tooltip * add 'hover' option to Tooltip * make front-end and back-end logic the same * add Alpine option when creating routes

vnugent · 2025-02-05T04:43:28Z

src/components/ui/Tooltip.tsx


 interface Props {
  content: string | ReactNode
  enabled?: boolean
  children: JSX.Element | JSX.Element [] | null
  className?: string
+  trigger?: 'click' | 'hover'


Let's not introduce a new behavior for Tooltip, and keep it consistent with existing UX (onClick show a tip, include a clickable external link if needed)

vnugent · 2025-02-05T11:19:37Z

@all-contributors add @glassbead0 for code and ideas

allcontributors · 2025-02-05T11:19:47Z

@vnugent

I've put up a pull request to add @glassbead0! 🎉

glassbead0 deployed to Preview January 24, 2025 23:59 — with GitHub Actions View deployment

glassbead0 mentioned this pull request Jan 25, 2025

feat: adding tick restriction for attempt type OpenBeta/openbeta-graphql#444

Merged

glassbead0 deployed to Preview January 26, 2025 01:26 — with GitHub Actions View deployment

glassbead0 commented Jan 26, 2025

View reviewed changes

glassbead0 force-pushed the restrict_attempt_type_for_ticks branch from 36b5e07 to 92de173 Compare January 26, 2025 01:36

glassbead0 deployed to Preview January 26, 2025 01:36 — with GitHub Actions View deployment

vnugent requested changes Jan 26, 2025

View reviewed changes

glassbead0 force-pushed the restrict_attempt_type_for_ticks branch from 92de173 to 6681de1 Compare January 26, 2025 20:20

glassbead0 deployed to Preview January 26, 2025 20:20 — with GitHub Actions View deployment

glassbead0 mentioned this pull request Jan 30, 2025

Can't choose 'Alpine' as a Discipline when creating a new route. #1261

Open

glassbead0 force-pushed the restrict_attempt_type_for_ticks branch from 6681de1 to 4edf891 Compare January 30, 2025 22:38

glassbead0 had a problem deploying to Preview January 30, 2025 22:39 — with GitHub Actions Failure

glassbead0 force-pushed the restrict_attempt_type_for_ticks branch from 4edf891 to 87e4b61 Compare January 30, 2025 23:13

glassbead0 had a problem deploying to Preview January 30, 2025 23:14 — with GitHub Actions Failure

glassbead0 commented Jan 30, 2025

View reviewed changes

glassbead0 force-pushed the restrict_attempt_type_for_ticks branch from 87e4b61 to af02ac0 Compare January 31, 2025 00:31

glassbead0 deployed to Preview January 31, 2025 00:32 — with GitHub Actions View deployment

glassbead0 requested a review from vnugent January 31, 2025 00:42

vnugent reviewed Feb 5, 2025

View reviewed changes

glassbead0 added 3 commits February 4, 2025 18:41

feat: Update tick logic to match backend

f65c0b0

feat: address PR comments

c8df804

* use phosphor icons * use existing Tooltip * add 'hover' option to Tooltip * make front-end and back-end logic the same * add Alpine option when creating routes

vnugent requested changes Feb 5, 2025

View reviewed changes

fix: remove hover tooltip option

a3f1ad9

glassbead0 force-pushed the restrict_attempt_type_for_ticks branch from af02ac0 to a3f1ad9 Compare February 5, 2025 05:18

glassbead0 deployed to Preview February 5, 2025 05:19 — with GitHub Actions View deployment

glassbead0 requested a review from vnugent February 5, 2025 05:23

vnugent merged commit d4a00ad into OpenBeta:develop Feb 5, 2025
3 checks passed

allcontributors bot mentioned this pull request Feb 5, 2025

docs: add glassbead0 as a contributor for code, and ideas #1273

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Restrict attemptType based on style selected for ticks #1260

feat: Restrict attemptType based on style selected for ticks #1260

glassbead0 commented Jan 24, 2025 •

edited

Loading

glassbead0 Jan 26, 2025

glassbead0 Jan 26, 2025

vnugent Jan 26, 2025

glassbead0 Jan 26, 2025

glassbead0 Jan 26, 2025

vnugent Jan 26, 2025

vnugent commented Jan 26, 2025

vnugent Jan 26, 2025

vnugent Jan 26, 2025

vnugent Jan 26, 2025

glassbead0 commented Jan 26, 2025

glassbead0 Jan 30, 2025

vnugent Feb 5, 2025

glassbead0 Feb 5, 2025

glassbead0 Jan 30, 2025

vnugent Feb 5, 2025

glassbead0 Feb 5, 2025 •

edited

Loading

vnugent Feb 5, 2025

vnugent Feb 5, 2025

vnugent Feb 5, 2025

vnugent commented Feb 5, 2025

allcontributors bot commented Feb 5, 2025

feat: Restrict attemptType based on style selected for ticks #1260

feat: Restrict attemptType based on style selected for ticks #1260

Conversation

glassbead0 commented Jan 24, 2025 • edited Loading

What type of PR is this?(check all applicable)

Description

Related Issues

What this PR achieves

Screenshots, recordings

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnugent commented Jan 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glassbead0 commented Jan 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glassbead0 Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnugent commented Feb 5, 2025

allcontributors bot commented Feb 5, 2025

glassbead0 commented Jan 24, 2025 •

edited

Loading

glassbead0 Feb 5, 2025 •

edited

Loading