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.

Sign up for GitHub

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

Jump to bottom

fix(modern-challenges): Default nav link to "Editor" if only one editor #16570

Merged
merged 1 commit into from
Jan 30, 2018
Merged

fix(modern-challenges): Default nav link to "Editor" if only one editor #16570

merged 1 commit into from
Jan 30, 2018

Conversation

werne2j
Copy link
Contributor

@werne2j werne2j commented Jan 25, 2018 •
edited
Loading

Pre-Submission Checklist

  • Your pull request targets the staging branch of freeCodeCamp.
  • Branch starts with either fix/, feature/, or translate/ (e.g. fix/signin-issue)
  • You have only one commit (if not, squash them into one commit).
  • All new and existing tests pass the command npm test. Use git commit --amend to amend any fixes.

Type of Change

  • Small bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Add new translation (feature adding new translations)

Checklist:

  • Tested changes locally.
  • Addressed currently open issue

Closes #16529

Description

For Modern Challenges when there is only one editor we want to default the nav link to display Editor instead of the filename. When there is more than one file then we will display the filename in the nav link

@BerkeleyTrue BerkeleyTrue added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Jan 25, 2018
BerkeleyTrue
BerkeleyTrue suggested changes Jan 25, 2018
View reviewed changes
common/app/routes/Challenges/views/Modern/Show.jsx Outdated
}
return {
nameToFileKey: _.reduce(files, (map, file) => {
map['Editor'] = file.key;

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@raisedadead raisedadead added status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels Jan 25, 2018
@camperbot
Copy link
Contributor

@werne2j updated the pull request.

BerkeleyTrue
BerkeleyTrue reviewed Jan 25, 2018
View reviewed changes
common/app/routes/Challenges/views/Modern/Show.jsx Outdated
})
files => {
if (Object.keys(files).length === 1) {
return { nameToFileKey: { Editor: files[Object.keys(files)[0]].key }};

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@camperbot
Copy link
Contributor

@werne2j updated the pull request.

1 similar comment
@camperbot
Copy link
Contributor

@werne2j updated the pull request.

no-stack-dub-sack
no-stack-dub-sack approved these changes Jan 27, 2018
View reviewed changes
Copy link
Member

@no-stack-dub-sack no-stack-dub-sack left a comment

Choose a reason for hiding this comment

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

thanks @werne2j, with @BerkeleyTrue's suggested changes, this LGTM, but I'll let him take a final look before merging this is.

BerkeleyTrue
BerkeleyTrue reviewed Jan 29, 2018
View reviewed changes
common/app/routes/Challenges/views/Modern/Show.jsx Outdated
[types.toggleSidePanel]: 'Lesson'
});
[types.toggleSidePanel]: 'Lesson',
[`${types.toggleModernEditor}(${getFirstFileKey(files)})`]: 'Editor'

This comment was marked as off-topic.

Sign in to view

@camperbot
Copy link
Contributor

@werne2j updated the pull request.

BerkeleyTrue
BerkeleyTrue suggested changes Jan 30, 2018
View reviewed changes
common/app/routes/Challenges/views/Modern/Show.jsx Outdated
@@ -18,18 +18,26 @@ import { filesSelector } from '../../../../files';
const createModernEditorToggleType = fileKey =>
types.toggleModernEditor + `(${fileKey})`;

const getFirstFileKey = object =>
_.flow(_.values, _.first, _.property('key'))(object);

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

common/app/routes/Challenges/views/Modern/Show.jsx Outdated
_.forEach(files, (file) => {
panesMap[createModernEditorToggleType(file.fileKey)] = file.name;
});
}

This comment was marked as off-topic.

Sign in to view

common/app/routes/Challenges/views/Modern/Show.jsx Outdated
return map;
}, {

let panesMap = {

This comment was marked as off-topic.

Sign in to view

@camperbot
Copy link
Contributor

@werne2j updated the pull request.

@BerkeleyTrue BerkeleyTrue added status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. and removed status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP labels Jan 30, 2018
@BerkeleyTrue BerkeleyTrue merged commit cbb7c72 into freeCodeCamp:staging Jan 30, 2018
@BerkeleyTrue BerkeleyTrue removed the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Jan 30, 2018
@BerkeleyTrue
Copy link
Contributor

💥 nice work!!

Happy Coding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BerkeleyTrue BerkeleyTrue BerkeleyTrue approved these changes

@no-stack-dub-sack no-stack-dub-sack no-stack-dub-sack approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@werne2j @camperbot @BerkeleyTrue @no-stack-dub-sack @raisedadead