#74 Stepper implementation for wizard
Merged by lbrabec. Opened by manishakanyal.
fedora-qa/ manishakanyal/landingpage ImproveWizardPage  into  master

Download 74.patch

This pr redesigns the Wizard page.

@lbrabec and @jskladan, please have a look.

@manishakanyal After a quick pass through the changes:

  • I find the next/previous buttons a bit "artificial" and annoying at the same time. I understand the motivation, but I'd much rather just be able to click on the option (e.g. several days / few hours) and be shown the next page. The navigation "back" could then be solved by breadcrumbs, so you'd be able to just click on the previous page at will.
  • I'd like to see the browser's Go Back One Page feature be working as expected (e.g. when I went through Few Hours > Fedora Release Validation and now see the various options, the Back in browser does not send me back to "choose between release vaidation and bodhi karma" but to the main page
  • Connected with the ^^^, I'd prefer for the wizard to be also changing the URL in such a way, that a link can be shared to any path/point in it up until the end of the task selection
  • Should we decide to go with this style of the wizard (I, personaly, am not a huge fan, but that's just a personal opinion), I'd like for it to be consistent up until the end (or as long as is viable) - for example, at the "programming path" you have the "language selection" in the "new" style, and then the next page is suddenly in the old style, similarly with the "fedora release testing" path, the Base/Cloud/Desktop/... should still be in the "new" style to keep it more consistent

4 new commits added

  • added icons
  • filter based implementation of wizard
  • Wizard page redesign
  • filter implementation

1 new commit added

  • removed react-select

@lbrabec and @jskladan, please review it.

3 new commits added

  • stepper implementation for wizard
  • improve UI
  • solved conflicts

2 new commits added

  • wizard stepper implementation
  • solved conflicts

@lbrabec and @jskladan, please review it.

Seems that all from @jskladan's feedback was addressed.

My feedback for the updated stepped wizard:

  • the third item in the breadcrumbs is always 'area', it would be better if it was 'Languages' for easyfixes, 'Bodhi karma' for bodhi karma and 'Test type' for release validation ('Test type' sound kinda weird to me, but I don't have a better idea).
  • I would change the second item from 'actions' to 'area' and use 'actions' for the last step instead of 'data'
  • personally, I'd prefer if the text in card-body was aligned to the left
  • could you list all the languages in easyfix cards? (found in ['extra_data']['languages']), some easyfixes have more that one

Overall I think we are heading in the right direction.
@jskladan do you have any feedback on the updated stepped wizard?

Once the UI/UX related issued are resolved, I'll continue to review the code itself.

1 new commit added

  • added languages to cardbody

First of all - I like this way more, excellent work right there!

For now, all my feedback will disregard the "Actions" stage of the wizard, since that will be best solved in a separate ticket, and will reqiure (IMO) a lot of thought and work to make it properly usable and informative.

  • the Click on any of the below card to see data of your choice text is IMO not necessary, seems intuitive enough to me, that you are supposed to be clicking around in order to navigate a web-page (especially when the cursor changes on hover)
  • in the url (and this is obviously an absolute nitpick) I'd rather see provider instead of card (i.e. http://localhost:3000/wizard/step3?duration=FEW_HOURS&provider=fedora_easy_karma) to keep the keys consistent with the underlying data, the user is, of course, oblivious to this, but keeping it a bit more "semantic" feels good to me personally :)
  • WRT the URL construction - since the order of the steps is always structured pretty much the same, would you be able to do http://localhost:3000/wizard/FEW_HOURS/fedora_manual_testing/Base instead of the http://localhost:3000/wizard/step4?duration=FEW_HOURS&card=fedora_manual_testing&area=Base?
    • also, maybe few_hours instead of FEW_HOURS
  • in the Easifix path, I'd like to see another step between the Languages and Actions with a list of projects and their descriptions in the spirit of the language selection step previously, or the testtype selection in the manual testing path
  • maybe remove the Nothing sparked joy? text, and make the None of the above looks interesting to you? We'd be happy if you looked around at https://whatcanidoforfedora.org/! smaller? It feels a bit overpovering ATM, and while we want to give it as an option, we'd much rather for that option to be subdued. Maybe it could be a bit more prominent in the "last" step, so you see it as an alternative, but during the process, I think we should try to "hide" it a bit.
  • in the breadcrumb navigation, I'd either stick to singular or plural nouns. To me, singular sound a bit less clumsy, but I'm no native speaker (Duration / Area / Language / Action), or (and I'm not sure whether that makes more or less sense) go with the "selected item name" (as in the url) instead of the "page name" (Few Hours / Easifix / Python / Actions)
    • I'm also not the biggest fan of Area should we stick to the "page name" way, not that I can immediately offer a better alternative, but it just seems rather non-descriptive. Absolutely a nitpick though.

Makes sense @manishakanyal? WDYT @lbrabec?

The motive behind giving a direct list of issues after choosing a programming language is to give users all possible issues a user can contribute to the effectiveness of the project.
Suppose I select a programming language, I get a list of issues, and looking at the description, I may move to the issue tracker directly. But if there’s a limited number of issues(related to a single project), I may need to go back and forth to navigate through different projects and be able to find something I can contribute to.

I believe breadcrumbs represent a way for users to navigate throughout the wizard step process. Changing it to selected options like easyfix/manual testing/etc instead of the step name will hinder the motive of using breadcrumbs for navigation.
Example: If A user selects few_hours on step1 and goes to step2, going back to step 1 will require clicking on few_hours instead of duration which will make it a bad UX.

In my opinion, we may choose different keywords for breadcrumbs and URL.

These are just my opinions, please let me know which to keep.
@lbrabec and @jskladan

Suppose I select a programming language, I get a list of issues, and looking at the description, I may move to the issue tracker directly.

There are couple of things need decoupling here, I think. As I said, I'll try to refrain from commenting directly on the "actions" page, as it is rather messy and not really the point of the current PR, as I see it.

On the other hand "I may move to the issue tracker directly" is not the whole truth here. Sure, you could do it, but the thing is, we are not showing all the tickets from the project at hand, but a curated list, so you'd have to find a way of "going to the issue tracker directly, and setting the filters in such a way, that only the relevant tickets are shown".

While this is absolutely doable, it smells of bad design and unnecessary problems, caused by the "per-git-provider" implementation needed. Sure, we could say we only have Pagure, Github, Bitbucket and Gitlab now, but what if the list grows in the future? What if we need to interface with couple versions of GItlab, and each has slightly different URL builder?

But if there’s a limited number of issues(related to a single project), I may need to go back and forth to navigate through different projects and be able to find something I can contribute to.

Sure, but I could say the same about any of the previous steps - why should I navigate between "Few Hours" and "Several Days" when there's only one thing in the "Several Days" and I might not have been interested in what I see there?

My POW (and I guess we'll have to see what @lbrabec thinks) is, that I'm more motivated by "what the project is" than "how many easyfixes the project has". In the current state, I have no clue what release-monitoring/anitya or RITlug/TigerOS is, and just the fact that a project "uses Python" does not mean I'm interested in getting my hands dirty with it. I'd much rather see the page describing what the projects I can work on are, than a random name, and a ticket title. That just does not give me any relevant information to make a decision.

I believe breadcrumbs represent a way for users to navigate throughout the wizard step process. Changing it to selected options like easyfix/manual testing/etc instead of the step name will hinder the motive of using breadcrumbs for navigation.

Sure, on the other hand, the text in the breadcrumbs at the moment is rather non-descript, and has no real tie to the page, other than a previous knowledge of "Yeah, I think I clicked Fedora Release Validation in the Area step...".

But once again, that is just how my brain works, and it could be made satisfactory by using better descriptors in the "location-style" breadcrumb.

Understood! lets see what are the views of @lbrabec.

just the fact that a project 'uses Python" does not mean I'm interested in getting my hands dirty with it. I'd much rather see the page describing what the projects I can work on are

Yes, this is exactly the reason why one more step between programming language selection and the relevant tickets is a good idea that should be implemented. The project description has to be somewhere and it doesn't make a sense to have it written in every issue card.

I think the approach Josef proposed is better, it would more closely correspond to breadcrumbs in file manager (note that there is no 'action' breadcrumb needed):

> start >
> start > few hours
> start > few hours > bodhi karma
> start 
> start > several days
> start > several days > easyfix
> start > several days > easyfix > python >
> start > several days > easyfix > python > fedora-infra/bodhi

This seems to be pretty self-explanatory as probably everyone that happens to use our wizard is familiar with navigating in file system tree:

  • when you click on 'easyfix' you'll get to a step where all the languages are shown
  • when you click on 'python' you'll get to list of all projects written in python
  • when you click on 'few hours' you'll see all the options that takes several hours
  • when you click on 'start' (our root in the file system analogy) you'll see the option to choose how much time you can spend

Ok got it, I'll implement the changes.

2 new commits added

  • added new step and changed item in breadcrumps
  • wizard UX/UI improve

Implemented the changes, please have a look @lbrabec and @jskladan

@manishakanyal Thanks! I like the direction we're taking, couple of things to note:

  • I'd like for the http://localhost:3000/wizard/step4?duration=Several_days&provider=easyfix&action=Python page to have the cards more in line with http://localhost:3000/wizard/step3?duration=few_hours&provider=fedora_manual_testing rather than the final "pick an action" step
  • I think the breadcrumbs would look better, if the first letters were uppercase ( start / several days / easyfix / ... -> Start / Several Days / Easyfix / ...
  • How about the different url construction I proposed in the previous comment https://pagure.io/fedora-qa/landingpage/pull-request/74#comment-151419 (reposting for clarity):
    • Since the order of the steps is always structured pretty much the same, would you be able to do http://localhost:3000/wizard/FEW_HOURS/fedora_manual_testing/Base like URLs instead of the http://localhost:3000/wizard/step4?duration=FEW_HOURS&card=fedora_manual_testing&area=Base model? Or at least get rid of the step[1,2,3,4, ...] bit?
  • Since the order of the steps is always structured pretty much the same, would you be able to do http://localhost:3000/wizard/FEW_HOURS/fedora_manual_testing/Base like URLs instead of the http://localhost:3000/wizard/step4?duration=FEW_HOURS&card=fedora_manual_testing&area=Base model? Or at least get rid of the step[1,2,3,4, ...] bit?

I spoke with Manisha about this on matrix chat and was told that it would require a big part of the logic to be redone, I don't think it is worth it to spend time on this right now. Maybe we could create an issue, so we don't forget about this and revisit the issue in the later part of internship.

  • I'd like for the http://localhost:3000/wizard/step4?duration=Several_days&provider=easyfix&action=Python page to have the cards more in line with http://localhost:3000/wizard/step3?duration=few_hours&provider=fedora_manual_testing rather than the final "pick an action" step

Hi @jskladan, by in line did you mean that 2 in a row or three in a row, can you please explain?

2 new commits added

  • wizard stepper implementation
  • solved conflicts

Hi @lbrabec and @jskladan, I have implemented the changes, please review.

Hi @jskladan, by in line did you mean that 2 in a row or three in a row, can you please explain?

Sorry, should have been clearer there - what I meant was "to look/work the same as".

Hi @lbrabec and @jskladan, I have implemented the changes, please review.

Looks fine to me!

Last couple of nitpicks:
* I'd remove the >_ icons from http://localhost:3000/wizard/step4?duration=Several_days&provider=easyfix&action=Python (and also icons from all the actions pages) - they serve no real function since those are all the same, and (to my eyes, at least) only add visual clutter
* It would be great if the breadcrumbs worked as actual links, so I could, for example, use a middle-mouse click on Fedora Manual Testing to open itin a new tab while keeping the page with current actions open

Other than that,

1 new commit added

  • added links in breadcrumbs

Implemented the changes, @lbrabec and @jskladan please review.

LGTM! I guess it's @lbrabec's turn to dig through the code now :)

1 new commit added

  • used path instead of queryParams

@lbrabec, @jskladan I have changed the url from query Params to path variables, please review.

1 new commit added

  • changed link in landing page

First of all, great job with the new wizard URLs, thanks!

Couple of nitpicks:

  • could you please convert all params in URL to lowercase ("Several_days" to "several_days", "Cloud"/"Base"/"Installation"/... to "cloud"/"base"/"installation"/...). I think that project names stay as is (i.e. mixed case: "RITlug/teleirc")
  • project names often contain "/" (e.g. "RITlug/TigerOS", "fedora-infra/bodhi", ...), I believe this could cause problems if we expanded the wizard to accept longer URLs than current "step5" (which could lead to bugs such as "fedora-infra" would become one param and "bodhi" another). Maybe you could escape the project names (see encodeURIComponent() or some other similar function).

Code review will follow in separate comments.

Do these three (plus one commented) routes serve any purpose? If not, please remove them.

This seems to be redundant, the same line is above.

I think this can be joined to one ternary operator with condition: query.issues || query.action || query.provider === "fedora_easy_karma"

This is quite repetitive, could you please create a new function createBreadcrumbItem(to, className) that would return appropriate BreadcrumbItem?

Also, does the onClick function do anything now? Maybe I am missing something, but since you are not pushing to history and returning string, I think you can omit the onClick function.

1 new commit added

  • changed all params in URL to lowercase

@lbrabec, I have implemented the changes, can you please review them?

2 new commits added

  • wizard stepper implementation
  • solved conflicts

rebased onto 79f1395861a400b394a293957535db2dc50b59af

Thanks!

Pull-Request has been merged by lbrabec

Metadata