Skip to content

Commit

Permalink
feat: handle rtl with attribute only (openedx#12)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: This package will no longer handle toggling rtl css on or off. Prefer to use a single file with rules scoped on [dir='rtl'] or [dir='ltr']
  • Loading branch information
Adam Butterworth authored Jun 3, 2019
1 parent d1bbfc3 commit 7653376
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 135 deletions.
101 changes: 47 additions & 54 deletions src/lib.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,47 +90,6 @@ addLocaleData([
...ukLocale,
]);

/**
* Configures the i18n library with messages for your application.
*
* The first is the application configuration object. The second parameter is an
* object containing messages for each supported locale, indexed by locale name.
*
* Example of second parameter:
*
* {
* en: {
* "message.key": "Message Value"
* },
* 'es-419': {
* "message.key": "Valor del mensaje"
* }
* ...
* }
*
* Logs a warning if it detects a locale it doesn't expect (as defined by the supportedLocales list
* above), or if an expected locale is not provided.
*/
export const configure = (newConfig, msgs) => {
validateConfiguration(newConfig);
messages = msgs;
config = newConfig;

if (config.ENVIRONMENT !== 'production') {
Object.keys(messages).forEach((key) => {
if (supportedLocales.indexOf(key) < 0) {
console.warn(`Unexpected locale: ${key}`); // eslint-disable-line no-console
}
});

supportedLocales.forEach((key) => {
if (messages[key] === undefined) {
console.warn(`Missing locale: ${key}`); // eslint-disable-line no-console
}
});
}
};

/**
* Some of our dependencies function on primary language subtags, rather than full locales.
* This function strips a locale down to that first subtag. Depending on the code, this
Expand Down Expand Up @@ -202,18 +161,52 @@ export const isRtl = locale => rtlLocales.includes(locale);
* is a RTL language.
*/
export const handleRtl = () => {
if (config.ENVIRONMENT === 'production') {
// Get external style sheets only. The app may add <style> based stylesheets.
const appStylesheets = [].slice.call(global.document.styleSheets)
.filter(element => element.href !== null);
if (isRtl(getLocale())) {
global.document.getElementsByTagName('html')[0].setAttribute('dir', 'rtl');
appStylesheets[0].disabled = true;
appStylesheets[1].disabled = false;
} else {
global.document.getElementsByTagName('html')[0].removeAttribute('dir');
appStylesheets[0].disabled = false;
appStylesheets[1].disabled = true;
}
if (isRtl(getLocale())) {
global.document.getElementsByTagName('html')[0].setAttribute('dir', 'rtl');
} else {
global.document.getElementsByTagName('html')[0].setAttribute('dir', 'ltr');
}
};

/**
* Configures the i18n library with messages for your application.
*
* The first is the application configuration object. The second parameter is an
* object containing messages for each supported locale, indexed by locale name.
*
* Example of second parameter:
*
* {
* en: {
* "message.key": "Message Value"
* },
* 'es-419': {
* "message.key": "Valor del mensaje"
* }
* ...
* }
*
* Logs a warning if it detects a locale it doesn't expect (as defined by the supportedLocales list
* above), or if an expected locale is not provided.
*/
export const configure = (newConfig, msgs) => {
validateConfiguration(newConfig);
messages = msgs;
config = newConfig;

if (config.ENVIRONMENT !== 'production') {
Object.keys(messages).forEach((key) => {
if (supportedLocales.indexOf(key) < 0) {
console.warn(`Unexpected locale: ${key}`); // eslint-disable-line no-console
}
});

supportedLocales.forEach((key) => {
if (messages[key] === undefined) {
console.warn(`Missing locale: ${key}`); // eslint-disable-line no-console
}
});
}

handleRtl();
};
109 changes: 28 additions & 81 deletions src/lib.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,63 +103,46 @@ describe('lib', () => {
});

describe('getLocale', () => {
it('should throw an error if called before configure has been called with messages', () => {
// For testing purposes, sets messages back to null, emulating a situation where
// configure wasn't called.
beforeEach(() => {
configure(
{
ENVIRONMENT: 'production',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'yum',
},
null,
{
'es-419': {},
de: {},
'en-us': {},
},
);
expect(() => {
getLocale();
}).toThrowError('getLocale called before configuring i18n. Call configure with messages first.');
});

describe('when configured', () => {
beforeEach(() => {
configure(
{
ENVIRONMENT: 'production',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'yum',
},
{
'es-419': {},
de: {},
'en-us': {},
},
);
});

it('should return a supported locale as supplied', () => {
expect(getLocale('es-419')).toEqual('es-419');
expect(getLocale('en-us')).toEqual('en-us');
});
it('should return a supported locale as supplied', () => {
expect(getLocale('es-419')).toEqual('es-419');
expect(getLocale('en-us')).toEqual('en-us');
});

it('should return the supported primary language tag of a not-quite-supported locale', () => {
expect(getLocale('de-de')).toEqual('de');
});
it('should return the supported primary language tag of a not-quite-supported locale', () => {
expect(getLocale('de-de')).toEqual('de');
});

it('should return en if the locale is not supported at all', () => {
expect(getLocale('oh-no')).toEqual('en');
});
it('should return en if the locale is not supported at all', () => {
expect(getLocale('oh-no')).toEqual('en');
});

it('should look up a locale in the language preference cookie if one was not supplied', () => {
getCookies().get = jest.fn(() => 'es-419');
expect(getLocale()).toEqual('es-419');
it('should look up a locale in the language preference cookie if one was not supplied', () => {
getCookies().get = jest.fn(() => 'es-419');
expect(getLocale()).toEqual('es-419');

getCookies().get = jest.fn(() => 'pl');
expect(getLocale()).toEqual('en');
getCookies().get = jest.fn(() => 'pl');
expect(getLocale()).toEqual('en');

getCookies().get = jest.fn(() => 'de-bah');
expect(getLocale()).toEqual('de');
});
it('should fallback to the browser locale if the cookie does not exist', () => {
getCookies().get = jest.fn(() => null);
expect(getLocale()).toEqual(global.navigator.language.toLowerCase());
});
getCookies().get = jest.fn(() => 'de-bah');
expect(getLocale()).toEqual('de');
});
it('should fallback to the browser locale if the cookie does not exist', () => {
getCookies().get = jest.fn(() => null);
expect(getLocale()).toEqual(global.navigator.language.toLowerCase());
});
});

Expand Down Expand Up @@ -207,45 +190,15 @@ describe('lib', () => {
});

describe('handleRtl', () => {
/* WARNING:
*
* These handleRtl tests may pollute the document used by other tests in this file.
* Right now it doesn't matter, but if WEIRD STUFF starts happening, we may need to figure
* out how to properly mock global.document using jest and jsdom - I couldn't figure out how.
* Some of the properties are read-only, so this setup is a bit of a hack.
*/

let setAttribute;
let removeAttribute;

beforeAll(() => {
// Allow us to modify stylesheets in these tests. The styleSheets object cannot normally
// be set.
Object.defineProperty(global.document, 'styleSheets', {
value: [
{ disabled: false, href: null }, // Should be ignored because it has no href
{ disabled: false, href: 'real href' },
{ disabled: false, href: 'real href' },
{ disabled: false, href: null }, // Should be ignored because it has no href
],
});
});

beforeEach(() => {
setAttribute = jest.fn();
removeAttribute = jest.fn();

global.document.getElementsByTagName = jest.fn(() => [
{
setAttribute,
removeAttribute,
},
]);

// We use the same two stylesheets for each test, so just set them back to their default
// disabled-ness.
global.document.styleSheets[0].disabled = false;
global.document.styleSheets[1].disabled = false;
});

it('should do the right thing for non-RTL languages', () => {
Expand All @@ -261,10 +214,7 @@ describe('lib', () => {
);

handleRtl();
expect(setAttribute).not.toHaveBeenCalled();
expect(removeAttribute).toHaveBeenCalledWith('dir');
expect(global.document.styleSheets[1].disabled).toBe(false);
expect(global.document.styleSheets[2].disabled).toBe(true);
expect(setAttribute).toHaveBeenCalledWith('dir', 'ltr');
});

it('should do the right thing for RTL languages', () => {
Expand All @@ -281,9 +231,6 @@ describe('lib', () => {

handleRtl();
expect(setAttribute).toHaveBeenCalledWith('dir', 'rtl');
expect(removeAttribute).not.toHaveBeenCalled();
expect(global.document.styleSheets[1].disabled).toBe(true);
expect(global.document.styleSheets[2].disabled).toBe(false);
});
});
});

0 comments on commit 7653376

Please sign in to comment.