feat: move template selection into modal UI #201

boyney123 · 2021-12-06T10:33:34Z

Description

Related to issue #198

Moved the template selection into a modal now rather than it's own page.

To make this work I had some make some minor changes:

Changes

Changes to the UI to fit inside modal
Allow modal to accept custom classes so I could make it wider by default (lots to render on this page, max-w-2xl was not big enough, looked weird).
Removed a bug where two templates were getting rendered at the same time (currently in production)

UI: When user does not select anything

UI: When user selects something

Selecting a template then clicking "Create" will do as expected and render the new AsyncAPI file into the studio.

Let me know what you think @smoya , @magicmatatjahu @mcturco , hopefully this fixes the problem.

netlify · 2021-12-06T10:33:41Z

✔️ Deploy Preview for modest-rosalind-098b67 ready!

🔨 Explore the source changes: 14273f1

🔍 Inspect the deploy log: https://app.netlify.com/sites/modest-rosalind-098b67/deploys/61fa9bafdd9a6b0007e98abb

😎 Browse the preview: https://deploy-preview-201--modest-rosalind-098b67.netlify.app/

smoya · 2021-12-06T10:44:46Z

Love it! Question: could we do a solid line instead of dash-style for the selected template so it becomes more visible (only for the selected template)?
Not sure if makes sense but for me it helps visualizing what I did select.

magicmatatjahu

First of all, awesome! Thanks for fast contribution! But I have two problems:

we shouldn't make active nav item in the navigation, when someone click on that nav item. It doesn't look good for me. We should only open modal.
we shouldn't indicates that we "create" new file by templates, but use template, atm we don't create new file but reuse templates, so please change all occurences of "creating file" to the "use" or "reuse" - I made suggestion for that :)

And also please propagate suggestion from @smoya

src/components/Modals/NewFileModal.tsx

mcturco · 2021-12-06T14:47:50Z

could we do a solid line instead of dash-style for the selected template

to add to this suggestion by @smoya, I would also prefer that the inactive template card have a solid line as well. The light gray alone will be enough for the user to know that it is inactive. :)

Co-authored-by: Maciej Urbańczyk <[email protected]>

…yney123/studio into moving-template-selection-into-modals

boyney123 · 2021-12-13T11:17:52Z

Thanks @smoya , @mcturco @magicmatatjahu

to add to this suggestion by @smoya, I would also prefer that the inactive template card have a solid line as well. The light gray alone will be enough for the user to know that it is inactive. :)

Thanks @smoya and @mcturco , good ideas. That has been changed.

we shouldn't make active nav item in the navigation, when someone click on that nav item. It doesn't look good for me. We should only open modal.

Thanks @magicmatatjahu , think I have fixed that returning false from Sidebar.tsx (is that alright?)

 {
      name: 'newFile',
      state: () => false,
      icon: <VscNewFile className="w-5 h-5" />,
    },

we shouldn't indicates that we "create" new file by templates, but use template, atm we don't create new file but reuse templates, so please change all occurences of "creating file" to the "use" or "reuse" - I made suggestion for that :)

Yeah good point mate. Changes made.

Let me know if you have any more thoughts folks 👍

mcturco · 2021-12-13T14:53:42Z

Hey @boyney123! I am still seeing the dashed lines on the inactive templates. The solid border on the selected template looks great!! 😄 I was thinking that it would look more uniform if we kept the borders solid for all of them, but if we don't think that looks good we don't have to do that!!

mcturco · 2021-12-13T14:55:36Z

Oh! Also I just noticed that the "beta" label is now black in the upper left where the logo is.

Not sure if that was intentional, but just wanted to point it out either way. 😄

magicmatatjahu · 2021-12-13T15:01:25Z

@mcturco Yeah, Fran created issue for that 😄 #209

magicmatatjahu · 2021-12-14T15:47:44Z

@boyney123

Thanks @magicmatatjahu , think I have fixed that returning false from Sidebar.tsx (is that alright?)

Yeah, it should works :)

boyney123 · 2021-12-16T10:10:55Z

I am still seeing the dashed lines on the inactive templates

Sorry @mcturco I forgot to push up my changes 😅 , try again 🤞 🙏

magicmatatjahu

LGTM!

magicmatatjahu · 2021-12-16T11:30:36Z

@boyney123 You should rebase your PR with master branch to resolve merge conflicts (package-lock.json) :)

smoya · 2021-12-16T12:12:35Z

@boyney123 You should rebase your PR with master branch to resolve merge conflicts (package-lock.json) :)

This is due to npm version mismatch. Just checkout package-lock.json from source (meaning should be exact same content).

More about this: asyncapi/parser-js#427

mcturco · 2021-12-16T15:41:50Z

Awesome @boyney123!! Looks great 🎉

sonarcloud · 2021-12-17T07:36:05Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
38.9% Duplication

boyney123 · 2021-12-17T07:37:40Z

Thanks folks.

Thanks @smoya , yeah I have no idea what is going on with the lock file, as nothing has changed, I just reused the file from the master branch as suggested.

magicmatatjahu · 2021-12-17T09:53:16Z

@boyney123 I bumped npm to v8 and from v7 there is a new version and structure of package-lock (you probably use npm v6), Sergio created issue for that asyncapi/parser-js#427 Please also fix the duplication of code - https://sonarcloud.io/component_measures?id=asyncapi_studio&metric=new_duplicated_lines_density&pullRequest=201&selected=asyncapi_studio%3Asrc%2Fcomponents%2FModals%2FNewFileModal.tsx&view=list - and I will accept :)

src/components/Modals/NewFileModal.tsx

boyney123 · 2022-01-26T12:21:11Z

Fixed duplication thanks for the review @magicmatatjahu and @BOLT04

Think it's ready for review again.

I bumped npm to v8 and from v7 there is a new version and structure of package-lock (you probably use npm v6)

Also another node.... I upgraded to npm 8.3.2 which makes the lock file better, but it has added "dev": true to some packages, is this expected? Any idea @magicmatatjahu ?

magicmatatjahu · 2022-01-26T14:14:48Z

@boyney123 That label is added to packages installed from devDependencies list :)

https://stackoverflow.com/questions/49809490/what-is-dev-true-in-package-lock-json-for

magicmatatjahu

Awesome!

I found two bugs:

when I don't have selected template I can click Use Template and it replaces my current AsyncAPI spec with empty content, you can test it :) We should disable the Use Template button when user doesn't select template.
another is described in the separate comment.

Just these two things for me aren't acceptable, rest is ok :)

src/components/Modals/NewFileModal.tsx

magicmatatjahu · 2022-01-26T14:35:54Z

src/components/Content.tsx

-            {viewType === 'visualiser' && <VisualiserTemplate />}
-          </SplitPane> 
-        }
+        {newFileEnabled && <NewFileModal />}


All our modals has that nice enter animation (like settings on the bottom left corner) but templates hasn't and here is a problem. You always destroy and create new React node when user will open that model. I think that we should fix that by:

Suggested change

{newFileEnabled && <NewFileModal />}

<NewFileModal />

but we have to "watch" for change newFileEnabled in the NewFileModal component and change show prop to the show={newFileEnabled}.

Co-authored-by: Maciej Urbańczyk <[email protected]>

sonarcloud · 2022-02-02T14:58:38Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

magicmatatjahu · 2022-02-02T15:02:19Z

@boyney123 I applied my suggestions :) Could you check if you accept it and we can merge it :)

boyney123 · 2022-02-02T15:12:59Z

Thank you @magicmatatjahu yeah looks OK to me, just checked your changes and happy with them.

I can't approve the PR as I'm the author so guess this comment is the approval 🤣👍

magicmatatjahu

LGTM! 😆

magicmatatjahu · 2022-02-02T15:24:56Z

@mcturco Do you accept current solution?

mcturco · 2022-02-02T15:30:41Z

@magicmatatjahu yes, I actually just tested it and was about to write a comment that it looks good to me!! 🎉

magicmatatjahu · 2022-02-02T15:33:34Z

/rtm

asyncapi-bot · 2022-02-02T15:49:35Z

🎉 This PR is included in version 0.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

moving template selection into modals

c647871

boyney123 mentioned this pull request Dec 6, 2021

Templates section button does not trigger events for closing or opening previous tabs #198

Closed

magicmatatjahu requested changes Dec 6, 2021

View reviewed changes

src/components/Modals/NewFileModal.tsx Outdated Show resolved Hide resolved

src/components/Modals/NewFileModal.tsx Outdated Show resolved Hide resolved

src/components/Modals/NewFileModal.tsx Outdated Show resolved Hide resolved

boyney123 and others added 6 commits December 6, 2021 14:53

Update src/components/Modals/NewFileModal.tsx

f1ce045

Co-authored-by: Maciej Urbańczyk <[email protected]>

Update src/components/Modals/NewFileModal.tsx

3c490da

Co-authored-by: Maciej Urbańczyk <[email protected]>

making small suggestion changes

4c1b74a

Merge branch 'moving-template-selection-into-modals' of github.com:bo…

f2801e6

…yney123/studio into moving-template-selection-into-modals

fixing merge

0b05d15

fixing merge

775c0f9

removing the dashed borders

87e79a7

merge

53754fd

magicmatatjahu previously approved these changes Dec 16, 2021

View reviewed changes

smoya mentioned this pull request Dec 16, 2021

[New Doc 📑]: specify which version of npm is required to run npm install. asyncapi/parser-js#427

Closed

1 task

David Boyne added 2 commits December 17, 2021 07:32

fixing merge

734045c

update to package-lock.json

d338186

boyney123 dismissed magicmatatjahu’s stale review via d338186 December 17, 2021 07:33

update to package-lock.json

404bb10

BOLT04 reviewed Dec 19, 2021

View reviewed changes

src/components/Modals/NewFileModal.tsx Outdated Show resolved Hide resolved

boyney123 and others added 2 commits January 26, 2022 11:34

Merge branch 'master' into moving-template-selection-into-modals

407c0c4

removed duplicate code

883566d

magicmatatjahu requested changes Jan 26, 2022

View reviewed changes

boyney123 and others added 4 commits January 27, 2022 13:51

Update src/components/Modals/NewFileModal.tsx

dfca533

Co-authored-by: Maciej Urbańczyk <[email protected]>

Merge branch 'master' into moving-template-selection-into-modals

9963493

apply fixes

b4f2869

apply fixes

14273f1

magicmatatjahu approved these changes Feb 2, 2022

View reviewed changes

asyncapi-bot added ready-to-merge autoapproved labels Feb 2, 2022

github-actions bot approved these changes Feb 2, 2022

View reviewed changes

asyncapi-bot merged commit ff1222c into asyncapi:master Feb 2, 2022

github-actions bot approved these changes Feb 2, 2022

View reviewed changes

asyncapi-bot added the released label Feb 2, 2022

magicmatatjahu mentioned this pull request Feb 14, 2022

For new users give them the ability to select a template file (or real world example) #147

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: move template selection into modal UI #201

feat: move template selection into modal UI #201

boyney123 commented Dec 6, 2021 •

edited

Loading

netlify bot commented Dec 6, 2021 •

edited

Loading

smoya commented Dec 6, 2021 •

edited

Loading

magicmatatjahu left a comment

mcturco commented Dec 6, 2021

boyney123 commented Dec 13, 2021

mcturco commented Dec 13, 2021

mcturco commented Dec 13, 2021

magicmatatjahu commented Dec 13, 2021

magicmatatjahu commented Dec 14, 2021

boyney123 commented Dec 16, 2021

magicmatatjahu left a comment

magicmatatjahu commented Dec 16, 2021

smoya commented Dec 16, 2021

mcturco commented Dec 16, 2021

sonarcloud bot commented Dec 17, 2021

boyney123 commented Dec 17, 2021

magicmatatjahu commented Dec 17, 2021 •

edited

Loading

boyney123 commented Jan 26, 2022

magicmatatjahu commented Jan 26, 2022

magicmatatjahu left a comment

magicmatatjahu Jan 26, 2022 •

edited

Loading

sonarcloud bot commented Feb 2, 2022

magicmatatjahu commented Feb 2, 2022

boyney123 commented Feb 2, 2022

magicmatatjahu left a comment

magicmatatjahu commented Feb 2, 2022

mcturco commented Feb 2, 2022

magicmatatjahu commented Feb 2, 2022

asyncapi-bot commented Feb 2, 2022

feat: move template selection into modal UI #201

feat: move template selection into modal UI #201

Conversation

boyney123 commented Dec 6, 2021 • edited Loading

Changes

UI: When user does not select anything

UI: When user selects something

netlify bot commented Dec 6, 2021 • edited Loading

smoya commented Dec 6, 2021 • edited Loading

magicmatatjahu left a comment

Choose a reason for hiding this comment

mcturco commented Dec 6, 2021

boyney123 commented Dec 13, 2021

mcturco commented Dec 13, 2021

mcturco commented Dec 13, 2021

magicmatatjahu commented Dec 13, 2021

magicmatatjahu commented Dec 14, 2021

boyney123 commented Dec 16, 2021

magicmatatjahu left a comment

Choose a reason for hiding this comment

magicmatatjahu commented Dec 16, 2021

smoya commented Dec 16, 2021

mcturco commented Dec 16, 2021

sonarcloud bot commented Dec 17, 2021

boyney123 commented Dec 17, 2021

magicmatatjahu commented Dec 17, 2021 • edited Loading

boyney123 commented Jan 26, 2022

magicmatatjahu commented Jan 26, 2022

magicmatatjahu left a comment

Choose a reason for hiding this comment

magicmatatjahu Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Feb 2, 2022

magicmatatjahu commented Feb 2, 2022

boyney123 commented Feb 2, 2022

magicmatatjahu left a comment

Choose a reason for hiding this comment

magicmatatjahu commented Feb 2, 2022

mcturco commented Feb 2, 2022

magicmatatjahu commented Feb 2, 2022

asyncapi-bot commented Feb 2, 2022

boyney123 commented Dec 6, 2021 •

edited

Loading

netlify bot commented Dec 6, 2021 •

edited

Loading

smoya commented Dec 6, 2021 •

edited

Loading

magicmatatjahu commented Dec 17, 2021 •

edited

Loading

magicmatatjahu Jan 26, 2022 •

edited

Loading