-
Notifications
You must be signed in to change notification settings - Fork 1
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
conditional disable button #132
Conversation
Visit the preview URL for this PR (updated for commit bc911e6): https://nwplus-io--pr132-phoenix-live-portal-beb8lkcd.web.app (expires Mon, 29 Nov 2021 01:20:15 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
components/NavBar.js
Outdated
${(p) => | ||
p.disabled && | ||
`&:hover { | ||
cursor: not-allowed; | ||
} | ||
}`} |
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 took a look at the preview link and the button is still disabled. A few suggestions:
- there could be a race condition between when the
isLive
flag is set and when theLivePortalButton
is rendered. I suggest settingisLive
tonull
by default in theuseState
, then only rendering theLivePortalButton
whenisLive
has been set (i.e.{isLive !== null && <LivePortalButton ... />}
) - a side note:
livePortalLink
is undefined when I run your branch locally - where is this collection being stored? might want to check if this is a field or a collection and if we're fetching from the right place. - minor UX enhancement: move the conditional
p.disabled
inside the hover pseudo class. For disabled=true, set cursor =not-allowed
, otherwise set it topointer
- (i.e.
p.disabled ? 'not-allowed' : 'pointer'
- (i.e.
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.
Lmk if you need any clarification. Also feel free to ping me if you want to pair on 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.
- live portal button is now only rendered when isLive is pulled from firebase
livePortalLink
no longer exists and I'll address this in another CMS ticket CMS: Add field to modify live portal link #133- conditionally change the pointer style done
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.
Lgtm! Thanks for setting this up!
Description
conditionally disable cursor for live portal
Other considerations
@panjenny0