Skip to content
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

Ticket #598 updated #639

Merged
merged 23 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
semi: false
singleQuote: true
bracketSpacing: false
printWidth: 100
trailingComma: es5
{
"semi": false,
"singleQuote": true,
"bracketSpacing": false,
"printWidth": 100,
"trailingComma": "es5"
}
10 changes: 5 additions & 5 deletions app/components/Article/article.css
Original file line number Diff line number Diff line change
Expand Up @@ -154,17 +154,17 @@ article .link-popup p {
gap: var(--spacing-32);
}

.form {
left: 845px;
}

article .footer-comtainer {
display: flex;
align-items: center;
margin-top: var(--spacing-56);
margin-bottom: var(--spacing-24);
gap: var(--spacing-16);
}

article .footer-comtainer > * {
margin: var(--spacing-8, 8px);
}

article a.see-more:not(.visible) + div.see-more-contents {
display: none;
}
Expand Down
32 changes: 23 additions & 9 deletions app/components/Article/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import KeepGoing from '~/components/Article/KeepGoing'
import CopyIcon from '~/components/icons-generated/Copy'
import EditIcon from '~/components/icons-generated/Pencil'
import Button, {CompositeButton} from '~/components/Button'
import Feedback from '~/components/Feedback'
import Feedback, {logFeedback} from '~/components/Feedback'
import {tagUrl} from '~/routesMapper'
import type {Glossary, Question, Banner as BannerType} from '~/server-utils/stampy'
import Contents from './Contents'
Expand All @@ -24,19 +24,33 @@ const ArticleFooter = (question: Question) => {
!isLoading(question) && (
<div className="footer-comtainer padding-bottom-40">
{date && <div className="grey"> {`Updated ${date}`}</div>}
<div className="flex-double">
<CompositeButton secondary>
<Button
className="secondary"
secondary
action={question.answerEditLink || ''}
tooltip="Suggest changes in Google Docs"
tooltip="Google Doc"
props={{target: '_blank', rel: 'noopener noreferrer'}}
>
<EditIcon className="no-fill" />
<EditIcon />
</Button>
</div>
<span>Was this page helpful?</span>

<Feedback pageid={question.pageid} labels />
</CompositeButton>
<Feedback
showForm
formClassName="form"
pageid={question.pageid}
onSubmit={async (message: string, option?: string) =>
logFeedback({
message,
option,
type: 'bot',
question: question.title || '',
answer: question.text || '',
})
}
options={['Unclear', 'Too wordy', 'Confusing', 'Incorrect', 'Other']}
upHint="This page was helpful"
downHint="This page was unhelpful"
/>
</div>
)
)
Expand Down
55 changes: 55 additions & 0 deletions app/components/Button/button.css
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
letter-spacing: -0.16px;
}

.button-secondary {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this differ from the .secondary class below? That is conceptually what's the difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second version of the button, as represented in the figma, is significantly stylistically different, mainly as a result of the difference in hover states (the main button shows and outline, and the second shows a background). This also effects how sibling buttons are grouped. The .secondary class is a slight difference to the main button version, and the .button-secondary class is a second version of the button.

position: relative;
padding: 0.25rem;
border-radius: var(--border-radius);
border-width: 0;
line-height: 0;
background: inherit;
}

/* style */

.primary {
Expand Down Expand Up @@ -154,6 +163,22 @@
border: 1px solid var(--colors-teal-200, #a6d9d7);
}

.button-secondary:hover .tool-tip-secondary {
opacity: 1;
}

.active path {
stroke: var(--colors-teal-600);
}

.active {
background-color: var(----colors-teal-50);
}

.inactive:hover {
background-color: var(--colors-light-grey);
}

/* #### Composite button #### */
.composite-button {
box-shadow: 0px var(--spacing-16) var(--spacing-40) 0px rgba(175, 183, 194, 0.2);
Expand Down Expand Up @@ -196,6 +221,20 @@
border-width: 1px;
}

.composite-button-secondary {
display: flex;
border-style: solid;
border-color: var(--colors-cool-grey-200);
border-width: 1px;
border-radius: var(--border-radius);
justify-content: center;
align-items: center;
padding: 7px;
gap: var(--spacing-4);
min-width: min-content;
min-height: min-content;
}

.tooltip::after {
content: attr(data-tooltip);
position: absolute;
Expand All @@ -209,6 +248,22 @@
font-size: 16px; /* hard to set via classes, what with this being a pseudoclass */
}

.tool-tip-secondary {
opacity: 0;
position: absolute;
top: -42px;
left: 50%;
transform: translateX(-50%);
background-color: #1b2b3e;
font-size: 14px;
color: #f2f2f2;
padding: 5px 15px;
border-radius: var(--spacing-8);
white-space: nowrap;
pointer-events: none;
line-height: var(--spacing-32);
}

.button.full-width {
width: 100%;
height: 48px;
Expand Down
36 changes: 30 additions & 6 deletions app/components/Button/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,32 @@ type ButtonProps = {
className?: string
tooltip?: string
disabled?: boolean
active?: boolean
secondary?: boolean
props?: {[k: string]: any}
}
const Button = ({children, action, tooltip, className, disabled = false, props}: ButtonProps) => {
const classes = ['button', className, tooltip && 'tooltip'].filter((i) => i).join(' ')
const Button = ({
children,
action,
secondary,
tooltip,
className,
disabled = false,
active = false,
props,
}: ButtonProps) => {
const classes = [
(secondary && 'button-secondary') || 'button',
className,
tooltip && !secondary && 'tooltip',
]
.filter((i) => i)
.join(' ')
if (typeof action === 'string') {
return (
<Link
to={action}
className={classes}
className={classes + ' ' + (secondary && (active ? 'active' : disabled ? '' : 'inactive'))}
data-tooltip={tooltip}
onClick={(e) => {
if (disabled) {
Expand All @@ -26,28 +43,35 @@ const Button = ({children, action, tooltip, className, disabled = false, props}:
{...props}
>
{children}
{secondary && tooltip && !disabled && <p className="tool-tip-secondary">{tooltip}</p>}
</Link>
)
}
return (
<button
className={classes}
className={classes + ' ' + (secondary && (active ? 'active' : disabled ? '' : 'inactive'))}
onClick={action}
data-tooltip={tooltip}
disabled={disabled}
{...props}
>
{children}
{secondary && tooltip && !disabled && <p className="tool-tip-secondary">{tooltip}</p>}
</button>
)
}

export interface CompositeButtonProps {
children: ReactNode
className?: string
secondary?: boolean
}
export const CompositeButton = ({children, className}: CompositeButtonProps) => (
<div className={`composite-button ${className || ''}`}>{children}</div>
export const CompositeButton = ({children, className = '', secondary}: CompositeButtonProps) => (
<div
className={`${(secondary ? 'composite-button-secondary' : 'composite-button') + ' ' + className}`}
>
{children}
</div>
)

export default Button
13 changes: 7 additions & 6 deletions app/components/Feedback/Form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,21 @@ import './feedback.css'
export type FeedbackFormProps = {
onSubmit?: (msg: string, option?: string) => Promise<any>
onClose?: () => void
className?: string
options?: string[]
}
const FeedbackForm = ({onSubmit, onClose, options}: FeedbackFormProps) => {
const FeedbackForm = ({onSubmit, onClose, options, className}: FeedbackFormProps) => {
const [selected, setSelected] = useState<string>()
const [message, setMessage] = useState('')
const [enabledSubmit, setEnabledSubmit] = useState(!options)
const [numClicks, setNumClicks] = useState(0)
const clickCheckerRef = useOutsideOnClick(onClose)

useEffect(() => {
// Hide the form after 10 seconds if the user hasn't interacted with it
// Hide the form after 15 seconds if the user hasn't interacted with it
const timeoutId = setInterval(() => {
onClose && onClose()
}, 10000)
}, 15000)

// Clear the timeout to prevent it from running if the component unmounts
return () => clearInterval(timeoutId)
Expand All @@ -39,7 +40,7 @@ const FeedbackForm = ({onSubmit, onClose, options}: FeedbackFormProps) => {
<div
ref={clickCheckerRef}
onClick={() => setNumClicks((current) => current + 1)}
className="col-5 feedback-form bordered"
className={'feedback-form bordered ' + (className ?? '')}
>
<span className="black small padding-bottom-32">What was the problem?</span>
{options?.map((option) => (
Expand All @@ -57,11 +58,11 @@ const FeedbackForm = ({onSubmit, onClose, options}: FeedbackFormProps) => {

<textarea
name="feedback-text"
className={['feedback-text bordered', !options ? 'no-options' : ''].join(' ')}
className={['feedback-text small bordered', !options ? 'no-options' : ''].join(' ')}
placeholder="Leave a comment (optional)"
onChange={(e) => setMessage(e.target.value)}
/>
<Button className="primary full-width" action={handleSubmit} disabled={!enabledSubmit}>
<Button className="primary full-width submit" action={handleSubmit} disabled={!enabledSubmit}>
<p>Submit feedback</p>
</Button>
</div>
Expand Down
43 changes: 34 additions & 9 deletions app/components/Feedback/feedback.css
Original file line number Diff line number Diff line change
@@ -1,23 +1,49 @@
.feedback .composite-button {
width: fit-content;
.feedback {
display: flex;
align-items: center;
}

.thanks {
transition: opacity 200ms ease-in-out;
margin-left: 0.5rem;
pointer-events: none;
}
.show {
opacity: 1;
}

.hide {
opacity: 0;
}

.feedback-form {
position: relative;
z-index: 2;
max-width: 384px;
padding: var(--spacing-32);
margin-top: var(--spacing-8);
display: flex;
flex-direction: column;
position: fixed;
top: 50px;
left: calc(13.333vw + 85px);
width: 384px;
padding: 30px;
border-radius: 6px;
}

/* Comment to Nemo when he reviews: why didn't you just do this the way you did the settings buttons? */
.select-option {
zarSou9 marked this conversation as resolved.
Show resolved Hide resolved
height: var(--spacing-48);
padding: var(--spacing-8) var(--spacing-16);
border-radius: var(--border-radius);
margin-bottom: var(--spacing-16);
cursor: pointer;
width: 100%;
}

.submit {
height: var(--spacing-48);
border-radius: var(--border-radius);
font-weight: 500 !important;
cursor: pointer;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@melissasamworth watch out, here she comes

width: 100%;
}

.select-option.selected {
border: 1px solid var(--colors-teal-500);
}
Expand All @@ -28,7 +54,6 @@
padding: var(--spacing-12);
margin: var(--spacing-40) 0;
box-sizing: content-box;
background: pink;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. interesting...

.feedback-text.no-options {
margin: var(--spacing-16) 0;
Expand Down
Loading
Loading