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.

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

Vertical stepper design #94

Merged
merged 62 commits into from
Feb 25, 2020
Merged

Vertical stepper design #94

merged 62 commits into from
Feb 25, 2020

Conversation

obulat
Copy link
Contributor

@obulat obulat commented Feb 20, 2020

Fixes #57

Description

This PR has three main goals:

  1. Implement the mockups of the design by Francisco with Vertical stepper and a Recommended license card on the right.
  2. Move state management to Vuex for easier handling of the app-wide state.
  3. Update the language, using the language approved by the CC team.

Other information

Checklist:

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the master branch of the repository.
  • My commit messages follow best practices.
  • I added tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no
    visible errors.

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

obulat added 30 commits February 6, 2020 16:04
Convert Help buttons into links; render links using 'modals' object and v-for loop

Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
…icense attributes have been selected

>> Fixes: When attribution is selected, CC BY is set as the selected license in the state. When Going back to step one, then selecting that you already know which license you need, CC BY is automatically set as the selected license in the dropdown. This creates some weird behavior where there is a selected license in the dropdown, but the "Next Question" button is not enabled.

Signed-off-by: Olga Bulat <[email protected]>
Copy link
Member

@akmadian akmadian left a comment

Choose a reason for hiding this comment

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

  • Step one title changes from "Do you know which license you need?" to "License Expertise" when advanced to step 2. I think it would be best to keep it as "Do you know which license you need?" for consistency.

  • Would be good to add cursor: pointer on step headers to show that they are clickable

  • Odd stepper behavior with ND licenses

    • Select "I know which license I need"
    • Select any license with ND
    • Select next
    • See behavior
  • A little thing, unrelated to the PR: the .env file in the repo's root contains stuff that is duplicated in the vue.config.js. Is it necessary to have .env?

@akmadian
Copy link
Member

Also would be good to add "fixes #57" in the PR body

@obulat
Copy link
Contributor Author

obulat commented Feb 23, 2020

  • Step one title changes from "Do you know which license you need?" to "License Expertise" when advanced to step 2. I think it would be best to keep it as "Do you know which license you need?" for consistency.
    This was the approved wording from the spreadsheet.
  • Would be good to add cursor: pointer on step headers to show that they are clickable
    Added.
  • Odd stepper behavior with ND licenses

    • Select "I know which license I need"
    • Select any license with ND
    • Select next
    • See behavior
      I've updated the method for setting visible/enabled steps which solved this problem.
  • A little thing, unrelated to the PR: the .env file in the repo's root contains stuff that is duplicated in the vue.config.js. Is it necessary to have .env?
    I'm not sure about that, these files were added automatically by vue-cli when it added i18n. Will need to look into it.

Copy link
Member

@akmadian akmadian left a comment

Choose a reason for hiding this comment

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

Looks great! I got pretty nit-picky again, but again, I think it's getting to be necessary at this point. Please address the comments and ask questions if needed :) I assumed you were ready for another review because you added a commit called "Build the site" and replied to my comments.

@akmadian akmadian added this to the Launch milestone Feb 24, 2020
Copy link
Member

@akmadian akmadian left a comment

Choose a reason for hiding this comment

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

Looks great! One small thing, and I think we should be ready to merge :)

Copy link
Member

@akmadian akmadian left a comment

Choose a reason for hiding this comment

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

This issue should be fixed before merge because it could potentially cause legal issues if the license deeds are inaccessible based on how we provide the information.

Copy link
Member

@akmadian akmadian left a comment

Choose a reason for hiding this comment

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

Looks great! I'm approving, but please do these two things before merging:

  • Change the site title in ./public/index.html from "Creative Commons License Chooser" to "Choose a License"
  • Please add a link to this repo in the footer, maybe something like "Contribute on GitHub." After "Icons by Noun Project"

Great work, I'm really excited about this :)

@obulat obulat merged commit 2efb594 into master Feb 25, 2020
@obulat obulat deleted the VerticalStepperDesign branch February 25, 2020 04:31
@obulat obulat restored the VerticalStepperDesign branch February 25, 2020 04:31
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.

UI Improvements
2 participants