diff --git a/website/src/featureFlags.ts b/website/src/featureFlags.ts index 33cc124cc3..1624d12e71 100644 --- a/website/src/featureFlags.ts +++ b/website/src/featureFlags.ts @@ -1,12 +1,5 @@ /* eslint-disable import/prefer-default-export */ -/** - * See: - * - https://github.com/nusmodifications/nusmods/issues/2057 - * - https://github.com/nusmodifications/nusmods/pull/3212 - */ -export const enableShortUrl = false; - /** Enable Course Planning Exercise */ export const enableCPEx = false; diff --git a/website/src/views/timetable/ShareTimetable.scss b/website/src/views/timetable/ShareTimetable.scss index d7367bc379..36c1e95492 100644 --- a/website/src/views/timetable/ShareTimetable.scss +++ b/website/src/views/timetable/ShareTimetable.scss @@ -1,4 +1,4 @@ -@import "~styles/utils/modules-entry.scss"; +@import '~styles/utils/modules-entry.scss'; .header { text-align: center; @@ -10,7 +10,7 @@ } h3 { - margin-bottom: 0.4rem; + margin: 0.4rem 0; font-weight: $font-weight-bold; font-size: 1.4rem; } @@ -27,11 +27,6 @@ margin-bottom: 2rem; } -.copyIcon { - // Cannot use .btn-svg because display: inline-flex messes up button groups - margin-bottom: -0.2rem; -} - .copyStatus { position: absolute; right: 0; @@ -53,6 +48,10 @@ text-align: center; } +.buttonContainer { + width: 58px; +} + .qrCode { max-width: 12rem; padding: 0.2rem; diff --git a/website/src/views/timetable/ShareTimetable.test.tsx b/website/src/views/timetable/ShareTimetable.test.tsx index e0a5246652..c0d5d1f9e0 100644 --- a/website/src/views/timetable/ShareTimetable.test.tsx +++ b/website/src/views/timetable/ShareTimetable.test.tsx @@ -1,7 +1,7 @@ import axios, { AxiosResponse } from 'axios'; import { shallow, ShallowWrapper } from 'enzyme'; +import { Maximize2, Minimize2 } from 'react-feather'; -import { enableShortUrl } from 'featureFlags'; import { waitFor } from 'test-utils/async'; import Modal from 'views/components/Modal'; import LoadingSpinner from 'views/components/LoadingSpinner'; @@ -33,7 +33,7 @@ describe('ShareTimetable', () => { }, }; - const openModal = (wrapper: ShallowWrapper) => wrapper.find('button').simulate('click'); + const openModal = (wrapper: ShallowWrapper) => wrapper.find('button').first().simulate('click'); const closeModal = (wrapper: ShallowWrapper) => wrapper.find(Modal).first().props().onRequestClose!({} as any); @@ -54,64 +54,55 @@ describe('ShareTimetable', () => { openModal(wrapper); expect(wrapper.find(Modal).exists()).toBe(true); - if (enableShortUrl) { - expect(mockAxios.get).toHaveBeenCalledTimes(1); - } else { - expect(mockAxios.get).toHaveBeenCalledTimes(0); - } + expect(mockAxios.get).toHaveBeenCalledTimes(1); }); - if (enableShortUrl) { - test('should cache short URL from the API', () => { - const wrapper = shallow( - , - ); - - // Open, close and open the modal again - openModal(wrapper); - closeModal(wrapper); - openModal(wrapper); - - // The second open should not cause a second call - expect(mockAxios.get).toHaveBeenCalledTimes(1); - closeModal(wrapper); - - // Changing the timetable should cause opening the modal to trigger another API call - wrapper.setProps({ timetable: { CS3216: { Lecture: '1' } } }); - expect(mockAxios.get).toHaveBeenCalledTimes(1); - openModal(wrapper); - expect(mockAxios.get).toHaveBeenCalledTimes(2); - closeModal(wrapper); - - // Changing the semester should also trigger another API call - wrapper.setProps({ semester: 2 }); - expect(mockAxios.get).toHaveBeenCalledTimes(2); - openModal(wrapper); - expect(mockAxios.get).toHaveBeenCalledTimes(3); - }); + test('should cache short URL from the API', () => { + const wrapper = shallow( + , + ); - test('should show spinner when loading', () => { - const wrapper = shallow( - , - ); + // Open, close and open the modal again + openModal(wrapper); + closeModal(wrapper); + openModal(wrapper); - openModal(wrapper); - expect(wrapper.find(LoadingSpinner).exists()).toBe(true); - }); - } + // The second open should not cause a second call + expect(mockAxios.get).toHaveBeenCalledTimes(1); + closeModal(wrapper); - test('should display shortUrl if available', async () => { + // Changing the timetable should cause opening the modal to trigger another API call + wrapper.setProps({ timetable: { CS3216: { Lecture: '1' } } }); + expect(mockAxios.get).toHaveBeenCalledTimes(1); + openModal(wrapper); + expect(mockAxios.get).toHaveBeenCalledTimes(2); + closeModal(wrapper); + + // Changing the semester should also trigger another API call + wrapper.setProps({ semester: 2 }); + expect(mockAxios.get).toHaveBeenCalledTimes(2); + openModal(wrapper); + expect(mockAxios.get).toHaveBeenCalledTimes(3); + }); + + test('should show spinner when loading', () => { + const wrapper = shallow( + , + ); + + openModal(wrapper); + expect(wrapper.find(LoadingSpinner).exists()).toBe(true); + }); + + test('should display shortUrl with show original url button if available', async () => { const wrapper = shallow( , ); await openAndWait(wrapper); - if (enableShortUrl) { - expect(wrapper.find('input').prop('value')).toEqual(MOCK_SHORTURL); - } else { - expect(wrapper.find('input').prop('value')).toBeTruthy(); - } + expect(wrapper.find('input').prop('value')).toEqual(MOCK_SHORTURL); + expect(wrapper.find(Maximize2).exists()).toBe(true); }); test('should display long URL if data is corrupted', async () => { @@ -122,6 +113,7 @@ describe('ShareTimetable', () => { await openAndWait(wrapper); + expect(wrapper.find('button').at(1).prop('disabled')).toBe(true); expect(wrapper.find('input').prop('value')).toBeTruthy(); }); @@ -133,6 +125,7 @@ describe('ShareTimetable', () => { await openAndWait(wrapper); + expect(wrapper.find('button').at(1).prop('disabled')).toBe(true); expect(wrapper.find('input').prop('value')).toBeTruthy(); }); @@ -155,4 +148,19 @@ describe('ShareTimetable', () => { expect(wrapper.find('input').prop('value')).toContain('hidden=CS1010S,CS1231S'); }); + + test('should change to original url and display shorten url button when clicked on show original url button', async () => { + const wrapper = shallow( + , + ); + + await openAndWait(wrapper); + expect(wrapper.find('input').prop('value')).toEqual(MOCK_SHORTURL); + expect(wrapper.find(Maximize2).exists()).toBe(true); + + wrapper.find('button').at(1).simulate('click'); + expect(wrapper.find(Maximize2).exists()).toBe(false); + expect(wrapper.find(Minimize2).exists()).toBe(true); + expect(wrapper.find('input').prop('value')).not.toBe(MOCK_SHORTURL); + }); }); diff --git a/website/src/views/timetable/ShareTimetable.tsx b/website/src/views/timetable/ShareTimetable.tsx index 74833ec47c..cc36361051 100644 --- a/website/src/views/timetable/ShareTimetable.tsx +++ b/website/src/views/timetable/ShareTimetable.tsx @@ -2,20 +2,20 @@ import * as React from 'react'; import axios from 'axios'; import classnames from 'classnames'; import qs from 'query-string'; -import { Copy, Mail, Repeat } from 'react-feather'; +import { Copy, Mail, Maximize2, Minimize2, Repeat } from 'react-feather'; import type { QRCodeSVG } from 'qrcode.react'; import type { SemTimetableConfig } from 'types/timetables'; import type { ModuleCode, Semester } from 'types/modules'; import config from 'config'; -import { enableShortUrl } from 'featureFlags'; import { absolutePath, timetableShare } from 'views/routes/paths'; import Modal from 'views/components/Modal'; import CloseButton from 'views/components/CloseButton'; import LoadingSpinner from 'views/components/LoadingSpinner'; import retryImport from 'utils/retryImport'; +import Tooltip from 'views/components/Tooltip'; import styles from './ShareTimetable.scss'; type CopyState = 'NOT_COPIED' | 'COPY_SUCCESS' | 'COPY_FAIL'; @@ -33,6 +33,9 @@ type State = { isOpen: boolean; urlCopied: CopyState; shortUrl: string | null; + fullUrl: string | null; + isFullUrl: boolean; + isLoading: boolean; }; function shareUrl( @@ -43,6 +46,22 @@ function shareUrl( return absolutePath(timetableShare(semester, timetable, hiddenModules)); } +function getToolTipContent(shortUrl: string | null, isFullUrl: boolean, isLoading: boolean) { + if (isLoading) { + return 'Shortening link'; + } + + if (!shortUrl) { + return 'Link shortener temporarily unavailable'; + } + + if (isFullUrl) { + return 'Shorten link'; + } + + return 'Show original link'; +} + // So that I don't keep typing 'shortUrl' instead export const SHORT_URL_KEY = 'shorturl'; @@ -59,6 +78,9 @@ export default class ShareTimetable extends React.PureComponent { isOpen: false, urlCopied: NOT_COPIED, shortUrl: null, + fullUrl: null, + isFullUrl: true, + isLoading: false, }; override componentDidMount() { @@ -70,14 +92,14 @@ export default class ShareTimetable extends React.PureComponent { } } - loadShortUrl = () => { + loadUrl = () => { const { semester, timetable, hiddenModules } = this.props; const url = shareUrl(semester, timetable, hiddenModules); // Don't do anything if the long URL has not changed if (this.url === url) return; - const showFullUrl = () => this.setState({ shortUrl: url }); + const showFullUrl = () => this.setState({ fullUrl: url, isFullUrl: true }); this.url = url; // Only try to retrieve shortUrl if the user is online @@ -86,27 +108,29 @@ export default class ShareTimetable extends React.PureComponent { return; } - this.setState({ shortUrl: null }); - - if (enableShortUrl) { - axios - .get('/api/shorturl', { params: { url }, timeout: 8000 }) - .then(({ data }) => { - if (data[SHORT_URL_KEY]) { - this.setState({ shortUrl: data[SHORT_URL_KEY] }); - } else { - showFullUrl(); - } - }) - // Cannot get short URL - just use long URL instead - .catch(showFullUrl); - } else { - showFullUrl(); - } + this.setState({ fullUrl: url, shortUrl: null, isFullUrl: true, isLoading: true }); + + axios + .get('/api/shorturl', { params: { url }, timeout: 8000 }) + .then(({ data }) => { + if (data[SHORT_URL_KEY]) { + this.setState({ + shortUrl: data[SHORT_URL_KEY], + isFullUrl: false, + isLoading: false, + }); + } else { + this.setState({ isLoading: false }); + } + }) + // Cannot get short URL - just use long URL instead + .catch(() => { + this.setState({ isLoading: false }); + }); }; openModal = () => { - this.loadShortUrl(); + this.loadUrl(); this.setState({ isOpen: true }); }; @@ -132,8 +156,17 @@ export default class ShareTimetable extends React.PureComponent { } }; - renderSharing(url: string) { + toggleShortenUrl = () => { + this.setState((prevState) => ({ + isFullUrl: !prevState.isFullUrl, + urlCopied: NOT_COPIED, + })); + }; + + renderSharing(fullUrl: string, shortUrl: string | null, isFullUrl: boolean, isLoading: boolean) { const { semester } = this.props; + const url = isFullUrl ? fullUrl : shortUrl ?? fullUrl; + const toggleUrlButton = isFullUrl ? : ; return (
@@ -144,6 +177,19 @@ export default class ShareTimetable extends React.PureComponent { ref={this.urlInput} readOnly /> + + + + +
@@ -214,7 +260,7 @@ export default class ShareTimetable extends React.PureComponent { } override render() { - const { isOpen, shortUrl } = this.state; + const { fullUrl, isLoading, isOpen, shortUrl, isFullUrl } = this.state; return ( <> @@ -222,8 +268,8 @@ export default class ShareTimetable extends React.PureComponent { type="button" className="btn btn-outline-primary btn-svg" onClick={this.openModal} - onMouseOver={this.loadShortUrl} - onFocus={this.loadShortUrl} + onMouseOver={this.loadUrl} + onFocus={this.loadUrl} > Share/Sync @@ -241,7 +287,7 @@ export default class ShareTimetable extends React.PureComponent {

- {shortUrl ? this.renderSharing(shortUrl) : } + {fullUrl && this.renderSharing(fullUrl, shortUrl, isFullUrl, isLoading)} );