Freecodecamp: Ambigous toggle buttons in Email Settings

[arjunv]

Issue Description

Inorder to disable or enable notification emails, the user is expected to toggle the options in the Email Settings page. However the toggle buttons are ambigous and does not clearly convey whether the On or Off implies that the emails are currently enabled or disabled. The fact that the unsubscribe links from emails errors out when not logged in does not help matters much either.

Suggested Solution

Buttons with text like Enabled/Disabled with suggestive colors might be the more user friendly way to go.

[prateekgoel]

I'll take up this.

[prateekgoel]

@erictleung How about using Yes/No instead of Enabled/Disabled?

[erictleung]

@prateekgoel yes/no sounds more intuitive :+1:

cc/ @QuincyLarson tagging you for comment on UI/UX changes :smile:

[prateekgoel]

@erictleung Great. 😄
@QuincyLarson Let me know if you have any thoughts regarding this.

[QuincyLarson]

@erictleung @prateekgoel I agree that this will be more clear.

I like how Medium does this. It's monochrome and would jive with freeCodeCamp's visual design. It also allows for more than an on/off state.

@prateekgoel would you be interested in updating this on staging? Also, could you Reactify this page? We'd like to overhaul our profile and settings pages to use components instead of Pug views.

[arjunv]

@prateekgoel With Yes/No or On/Off, the user will not be able to understand the current state. If we go with one of those, it wouldn't be clear whether On implies Notifications are currenly enabled or clicking will enable notifications.
Hence I went for something like Enabled (if the button implies that notifications are currently enabled). And if the button changes state to enable notifications, then the text should be Enable

@QuincyLarson My way sucks. This would be the right way to go!

[prateekgoel]

@QuincyLarson I wanted to help in this but I am new to React. So i think it would be good if someone else take this.

[ZacharyKearns]

@QuincyLarson I will take this up. It looks like the settings page is already using React.

[vs1682]

@ZacharyKearns you can check for ToggleButtonGroup and ToggleButton in react-bootstrap.

Note: we are currently using react-bootstrap v0.30.5 but ToggleButtonGroup and ToggleButton have been exported for public use only in v0.31.2

[ZacharyKearns]

@vs1682 v0.30.5 does not have ToggleButtonGroup or ToggleButton. React-bootstrap would need to be upgraded or a custom component would have to be made.

[vs1682]

@ZacharyKearns right, that is what I mentioned in my earlier comment.

[dhcodes]

tagging @BerkeleyTrue for thoughts on being able to upgrade react-bootstrap?

[BerkeleyTrue]

@dhcodes that version is currently within our semvar range although we should explicitly upgrade to that version and use the tilde instead of the caret in our package.json

[dhcodes]

Submitted a PR to update react-bootstrap 🎉 . You'll need to add ToggleButtonGroup and ToggleButton to the import on the Settings.jsx file when you go to make this change.

[vs1682]

Nice.
@ZacharyKearns your turn.

[ZacharyKearns]

@vs1682 @dhcodes Okay great. I am just doing a coding challenge for a potential new job. I can start on this right after that.

[ZacharyKearns]

I have started working on this. Here a screenshot of what I have so far.

They do break at certain screen widths

Still working on making them functional

[dhcodes]

@ZacharyKearns Awesome! Would <ToggleButtonGroup> styles have a display: block fix the breaking issue?

[ZacharyKearns]

@dhcodes I tried add block={ true } to the ToggleButtonGroup component with no luck. It appears that it's column gets to small which causes it to break. I am experimenting with different column layouts.

[vs1682]

@ZacharyKearns You can also try changing bsSize on <ToggleButtonGroup> for smaller screen widths.

Note: All buttons on this page have large size. Changing bsSize will make <ToggleButton> appear smaller than others. We should confirm if we can do that.

[ZacharyKearns]

@vs1682 Changing them to a smaller size did not stop them from breaking unfortunately.

@BerkeleyTrue Here are screenshots for the final version as well as the css that was added to the main.less file and one of the ToggleButtonGroup's. The new code passed all the tests.

Just want to note that there seems to be a major issue with the layout of the settings page.

Because the content is not wrapped in a traditional bootstrap container the negative margins on the rows are causing the elements to be 30px wider than the width of the viewport which is causing horizontal scrolling.

.email-settings {
  @media (max-width: 768px) {
    text-align: center;
  }
}

.toggle-btn-group {
  float: right;
  @media (max-width: 768px) {
    float: none;
    .btn-toggle {
      margin-top: 15px;
      margin-bottom: 25px;
    }
  }
}

<ToggleButtonGroup
  className='toggle-btn-group'
  name='montly-email'
  onChange={ toggleMonthlyEmail }
  type='radio'
  >
  <ToggleButton
    bsSize='lg'
    bsStyle='primary'
    className={
      classnames(
        'positive-20',
        { active: sendMonthlyEmail },
        'btn-toggle'
      )
    }
    type='radio'
    value={ 1 }
    >
    On
  </ToggleButton>
  <ToggleButton
    bsSize='lg'
    bsStyle='primary'
    className={
      classnames(
        'positive-20',
        { active: !sendMonthlyEmail },
        'btn-toggle'
      )
    }
    type='radio'
    value={ 2 }
    >
    Off
  </ToggleButton>
</ToggleButtonGroup>

This view is for tablets and desktops

This view is for smartphones

If there are any problems please let me know before I issue a pull request.

This is my first contribution so be gentle haha.

[dhcodes]

@ZacharyKearns Nice work! If adding a wrapping makes the layout better, I'd say you might as well make that change in your PR too.

[ZacharyKearns]

Thanks @dhcodes

Adding a div with the container class seems to fix the horizontal scrolling so I will add it to the PR.

There is still work to be done on this page.

The job search settings button is also broken. Maybe that should be it's own issue?