Skip to content

feat: Made booker layout settings more user friendly#9397

Merged
PeerRich merged 3 commits intomainfrom
feat/smarter-booker-layout-settings
Jun 8, 2023
Merged

feat: Made booker layout settings more user friendly#9397
PeerRich merged 3 commits intomainfrom
feat/smarter-booker-layout-settings

Conversation

@JeroenReumkens
Copy link
Copy Markdown
Contributor

@JeroenReumkens JeroenReumkens commented Jun 7, 2023

What does this PR do?

Makes booker layout settings a bit more user friendly. These things have changed:

  1. When you disable a layout as one of the enabled options, the button to mark it as default layout disables too.
  2. If it was chosen as a default before you disabled that layout, it takes the next enabled layout and makes that default now.
  3. If you only enable one layout, the settings for the default layout aren't shown at all

Fixes CAL-1889 CAL-1893 CAL-1894 CAL-1895

CleanShot.2023-06-07.at.18.34.22.mp4

Type of change

  • New feature (non-breaking change which adds functionality)

How should this be tested?

  • Enable and disable layouts, and ensure that the behaviour is clear.
  • Test that the enabled layouts are also still shown correctly on the booker page
  • Test with a user that doesn't have any settings set yet, and note that by default it now only shows the month view.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 7, 2023 6:19pm
web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 7, 2023 6:19pm
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api ⬜️ Ignored (Inspect) Jun 7, 2023 6:19pm
ui ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 6:19pm

@linear
Copy link
Copy Markdown

linear Bot commented Jun 7, 2023

CAL-1893 By default if no settings are set, we only want to see monthly view. This way not everyone gets the new layout without changing something themselves.

CAL-1894 Disabled buttons for default layouts if that layout is not enabled

CAL-1889 Layout toggle improvements

  • By default if no settings are set, we only want to see monthly view. This way not everyone gets the new layout without changing something themselves.
  • Disabled buttons for default layouts if that layout is not enabled
  • Hide default layout selection completely when you only enable one layout.

CAL-1895 Hide default layout selection completely when you only enable one layout.

form={formMethods}
handleSubmit={(values) => {
const layoutError = validateBookerLayouts(values.defaultBookerLayouts || null);
const layoutError = validateBookerLayouts(values?.metadata?.defaultBookerLayouts || null);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is an unrelated issue. I noticed that I didn't change the fieldname after I moved the layout settings to metadata, so client side validation wasn't working. It is now.

defaultLayout: BookerLayouts.MONTH_VIEW,
enabledLayouts: bookerLayoutOptions,
};
const bookerLayouts = event.data?.profile?.bookerLayouts || defaultBookerLayoutSettings;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Instead of repeating the default settings I've moved it into a reusable variable.

// Converts the settings array into a boolean object, which can be used as form values.
const toggleValues: BookerLayoutState = bookerLayoutOptions.reduce((layouts, layout) => {
layouts[layout] = !shownSettings?.enabledLayouts
? true
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before if no settings were set, all layouts would be enabled. Now we only enable the layouts that are enabled in the default settings (so only month view as of now).

enabledLayouts: newEnabledLayouts,
// If default layout is toggled off, we set the default layout to the first enabled layout
// if there's none enabled, we set it to month view.
defaultLayout: isDefaultLayoutToggledOff
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added behaviour that if you disable the layout that's currently marked as the default, that we switch to the next best candidate. Otherwise you would have a layout marked as default that's actually a disabled radio button.

{bookerLayoutOptions.map((layout) => (
<RadioGroup.Item
className="aria-checked:bg-emphasis hover:bg-subtle focus:bg-subtle w-full rounded-[4px] p-1 text-sm transition-colors"
className="aria-checked:bg-emphasis hover:[&:not(:disabled)]:bg-subtle focus:[&:not(:disabled)]:bg-subtle w-full rounded-[4px] p-1 text-sm transition-colors disabled:cursor-not-allowed disabled:opacity-40"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Style changes to not show hover effects for disabled buttons.

let isPremiumUsername = false;

const layoutError = validateBookerLayouts(input?.metadata?.defaultBookerLayouts || null);
if (layoutError) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There was now backend validation yet for the booker layouts in the update profile handler. There is now.

<RadioGroup.Item
className="aria-checked:bg-emphasis hover:bg-subtle focus:bg-subtle w-full rounded-[4px] p-1 text-sm transition-colors"
className="aria-checked:bg-emphasis hover:[&:not(:disabled)]:bg-subtle focus:[&:not(:disabled)]:bg-subtle w-full rounded-[4px] p-1 text-sm transition-colors disabled:cursor-not-allowed disabled:opacity-40"
disabled={toggleValues[layout] === false}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This part disables this layout for being a default layout, if the layout itself is not enabled.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 7, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Jun 7, 2023

Current Playwright Test Results Summary

✅ 107 Passing - ⚠️ 4 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 06/07/2023 06:15:37pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: d5922a1

Started: 06/07/2023 06:13:28pm UTC

⚠️ Flakes

📄   apps/web/playwright/auth/delete-account.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Can delete user account
Retry 1Initial Attempt
0.80% (3) 3 / 376 runs
failed over last 7 days
3.72% (14) 14 / 376 runs
flaked over last 7 days

📄   apps/web/playwright/login.2fa.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
2FA Tests should allow a user to enable 2FA and login using 2FA
Retry 1Initial Attempt
0.54% (2) 2 / 372 runs
failed over last 7 days
18.28% (68) 68 / 372 runs
flaked over last 7 days
2FA Tests should allow a user to disable 2FA
Retry 1Initial Attempt
0.54% (2) 2 / 370 runs
failed over last 7 days
19.73% (73) 73 / 370 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 1Initial Attempt
6.61% (15) 15 / 227 runs
failed over last 7 days
93.39% (212) 212 / 227 runs
flaked over last 7 days

View Detailed Build Results


@JeroenReumkens JeroenReumkens changed the title Made booker layout settings more user friendly feat: Made booker layout settings more user friendly Jun 7, 2023
@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Jun 7, 2023

Finally an important change: After discussing with @Jaibles we concluded that it might not be the best experience if ALL users suddenly have all layouts enabled on their booking pages after release. So instead of enabling all layouts by default when you have no setting, we actually only show the month view like in the old booker, until you manually override settings either in the event settings or in your user settings.

this has been undone right?

ideally everyone has all 3 layouts and its opt-out to remove them

@ciaranha
Copy link
Copy Markdown
Member

ciaranha commented Jun 7, 2023

Finally an important change: After discussing with @Jaibles we concluded that it might not be the best experience if ALL users suddenly have all layouts enabled on their booking pages after release. So instead of enabling all layouts by default when you have no setting, we actually only show the month view like in the old booker, until you manually override settings either in the event settings or in your user settings.

this has been undone right?

ideally everyone has all 3 layouts and its opt-out to remove them

as far as I know, yes.

@JeroenReumkens
Copy link
Copy Markdown
Contributor Author

Finally an important change: After discussing with @Jaibles we concluded that it might not be the best experience if ALL users suddenly have all layouts enabled on their booking pages after release. So instead of enabling all layouts by default when you have no setting, we actually only show the month view like in the old booker, until you manually override settings either in the event settings or in your user settings.

this has been undone right?

ideally everyone has all 3 layouts and its opt-out to remove them

Correct. This isn't in there anymore. Let me update the description.


const layoutError = validateBookerLayouts(input?.metadata?.defaultBookerLayouts || null);
if (layoutError) {
const t = await getTranslation("en", "common");
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar Jun 8, 2023

Choose a reason for hiding this comment

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

why not const t = await getTranslation(ctx.user.locale ?? "en", "common");
?

Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

@JeroenReumkens LGTM great work. Left just one comment

@ciaranha
Copy link
Copy Markdown
Member

ciaranha commented Jun 8, 2023

@Udit-takkar is this good to merge or does your comment need addressing? If so can we jump in for @JeroenReumkens to address?

@Udit-takkar
Copy link
Copy Markdown
Contributor

@Jaibles It is good to merge. he can address the comment later

@PeerRich PeerRich added this pull request to the merge queue Jun 8, 2023
Merged via the queue into main with commit 93fc036 Jun 8, 2023
@PeerRich PeerRich deleted the feat/smarter-booker-layout-settings branch June 8, 2023 12:44
azadnsu pushed a commit to azadnsu/cal.com that referenced this pull request Jun 8, 2023
* Made booker layout settings more user friendly

* Fixed tyeps and default values in appearance form for booker layout

* Enable all layouts by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants