Fixes #29964 - Add memory allocation react component by yifatmakias · Pull Request #7794 · theforeman/foreman

yifatmakias · 2020-07-01T12:47:37Z

In order to eventually replace the jquery-ui-spinner, there is a need to add a component for the memory allocation input in a form.
This issue is for the addition of the component.

There are still some issues regarding this PR that should be dealt with.

houndci-bot · 2020-07-01T12:47:42Z

Unexpected missing end-of-source newline no-missing-end-of-source-newline

theforeman-bot · 2020-07-01T12:47:51Z

Issues: #29964

xprazak2 · 2020-07-02T09:40:42Z

I do not see it being used anywhere...

xprazak2 · 2020-07-02T09:40:45Z

nit: maybe useState(MBFormat) since it is defined a couple of lines above?

xprazak2 · 2020-07-02T09:42:00Z

Nice, seems to work as expected.

yifatmakias · 2020-07-16T11:55:12Z

[test foreman]

yifatmakias · 2020-07-16T12:49:57Z

[test foreman]

yifatmakias · 2020-07-19T12:49:16Z

[test foreman]

sharvit · 2020-08-02T12:19:10Z

+    "emotion-theming": "^10.0.27",
    "intl": "~1.2.5",
    "jed": "^1.1.1",
+    "rc-input-number": "^5.1.0",


Looks like they just released v6 a few days ago

theforeman-bot · 2020-08-03T13:52:29Z

@yifatmakias, this pull request is currently not mergeable. Please rebase against the develop branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream develop

This message was auto-generated by Foreman's prprocessor

yifatmakias · 2020-09-09T09:53:21Z

[test katello]

yifatmakias · 2020-09-10T06:22:03Z

[test katello]

sharvit

Thanks @yifatmakias, looks good and works as expected 👍

MariaAga · 2020-10-20T07:54:23Z

Looks like this snap is the same as the default props snap. This one should have an error message

I checked the error and warning states in a different way. Let me know if it looks ok now.

sharvit · 2020-10-20T07:54:25Z

@amirfefer did @yifatmakias addressed all your comments?

yifatmakias · 2020-10-21T11:38:20Z

[test foreman]

MariaAga

Looks good! Can you rebase and add resize and 4xl to the list of approved words in .eslintrc?

foreman/.eslintrc

Lines 13 to 14 in 6ed930a

    
           "skipWords": [ 
        
             "2xl",

yifatmakias · 2020-10-22T09:10:56Z

@MariaAga I rebased and added to .eslintrc file. let me know if it is ok :)

MariaAga

Awesome!

amirfefer

@yifatmakias Looks and works great 👍
just a few small issues

amirfefer · 2020-10-22T12:07:39Z

nit: An inequation between a number with a null gives an odd result i.e 5 > null returns true, you can find an interesting explanation here, therefore changing the default props values (for numbers) to undefined instead of null would be better.

5 > null // true, that's why we need maxValue != null 5 < null // false 5 > undefined // false, here we don't need the maxValue != null 5 < undefined // false

amirfefer · 2020-10-22T12:10:07Z

validationState looks redundant here, it isn't being used in this effect anyway

amirfefer

Thanks, @yifatmakias 👍 awesome feature
tested and works as expected

houndci-bot reviewed Jul 1, 2020

View reviewed changes

theforeman-bot added the UI label Jul 1, 2020

theforeman-bot added Needs testing Not yet reviewed labels Jul 1, 2020

yifatmakias force-pushed the 29964 branch from 37e5249 to 15bb881 Compare July 1, 2020 12:51

yifatmakias marked this pull request as draft July 1, 2020 13:29

yifatmakias force-pushed the 29964 branch from 15bb881 to 4f97334 Compare July 1, 2020 13:48

xprazak2 reviewed Jul 2, 2020

View reviewed changes

Comment thread package.json Outdated

xprazak2 Jul 2, 2020

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

I do not see it being used anywhere...

xprazak2 reviewed Jul 2, 2020

View reviewed changes

yifatmakias closed this Jul 2, 2020

yifatmakias reopened this Jul 2, 2020

tbrisker assigned xprazak2 Jul 8, 2020

yifatmakias force-pushed the 29964 branch 4 times, most recently from 99abf7d to 01106e6 Compare July 16, 2020 07:53

yifatmakias force-pushed the 29964 branch from 01106e6 to 3be1001 Compare July 19, 2020 12:16

sharvit reviewed Aug 2, 2020

View reviewed changes

yifatmakias force-pushed the 29964 branch from 3be1001 to 77cea73 Compare August 3, 2020 13:51

yifatmakias marked this pull request as ready for review August 3, 2020 13:52

theforeman-bot removed Needs testing Not yet reviewed labels Aug 3, 2020

theforeman-bot added the Waiting on contributor label Aug 3, 2020

yifatmakias changed the title ~~Fixes #29964 - Add memory allocation react component - WIP~~ Fixes #29964 - Add memory allocation react component Aug 3, 2020

yifatmakias force-pushed the 29964 branch from fa6dcbd to 6c1610f Compare September 9, 2020 08:08

yifatmakias requested a review from amirfefer October 13, 2020 08:17

sharvit previously approved these changes Oct 20, 2020

View reviewed changes

theforeman-bot removed the Needs re-review label Oct 20, 2020

MariaAga reviewed Oct 20, 2020

View reviewed changes

yifatmakias dismissed sharvit’s stale review via 7acc797 October 21, 2020 07:29

yifatmakias force-pushed the 29964 branch from 6c1610f to 7acc797 Compare October 21, 2020 07:29

yifatmakias force-pushed the 29964 branch from 7acc797 to f2f0c2d Compare October 22, 2020 06:30

MariaAga requested changes Oct 22, 2020

View reviewed changes

theforeman-bot added the Waiting on contributor label Oct 22, 2020

yifatmakias force-pushed the 29964 branch from f2f0c2d to 6a0e8ec Compare October 22, 2020 09:08

theforeman-bot added Needs re-review and removed Waiting on contributor labels Oct 22, 2020

MariaAga previously approved these changes Oct 22, 2020

View reviewed changes

theforeman-bot removed the Needs re-review label Oct 22, 2020

amirfefer suggested changes Oct 22, 2020

View reviewed changes

theforeman-bot added the Waiting on contributor label Oct 22, 2020

Fixes #29964 - Add memory allocation react component

7bbd8b9

yifatmakias dismissed MariaAga’s stale review via 7bbd8b9 October 22, 2020 18:26

yifatmakias force-pushed the 29964 branch from 6a0e8ec to 7bbd8b9 Compare October 22, 2020 18:26

theforeman-bot added Needs re-review and removed Waiting on contributor labels Oct 22, 2020

amirfefer approved these changes Oct 23, 2020

View reviewed changes

theforeman-bot removed the Needs re-review label Oct 23, 2020

amirfefer merged commit 3629fba into theforeman:develop Oct 23, 2020

Conversation

yifatmakias commented Jul 1, 2020

Uh oh!

houndci-bot Jul 1, 2020

Choose a reason for hiding this comment

Uh oh!

theforeman-bot commented Jul 1, 2020

Uh oh!

xprazak2 Jul 2, 2020

Choose a reason for hiding this comment

Uh oh!

xprazak2 Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xprazak2 commented Jul 2, 2020

Uh oh!

yifatmakias commented Jul 16, 2020

Uh oh!

yifatmakias commented Jul 16, 2020

Uh oh!

yifatmakias commented Jul 19, 2020

Uh oh!

sharvit Aug 2, 2020

Choose a reason for hiding this comment

Uh oh!

theforeman-bot commented Aug 3, 2020

Uh oh!

yifatmakias commented Sep 9, 2020

Uh oh!

yifatmakias commented Sep 10, 2020

Uh oh!

sharvit left a comment

Choose a reason for hiding this comment

Uh oh!

MariaAga Oct 20, 2020

Choose a reason for hiding this comment

Uh oh!

yifatmakias Oct 21, 2020

Choose a reason for hiding this comment

Uh oh!

sharvit commented Oct 20, 2020

Uh oh!

yifatmakias commented Oct 21, 2020

Uh oh!

MariaAga left a comment

Choose a reason for hiding this comment

Uh oh!

yifatmakias commented Oct 22, 2020

Uh oh!

MariaAga left a comment

Choose a reason for hiding this comment

Uh oh!

amirfefer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amirfefer Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amirfefer Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

amirfefer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

xprazak2 Jul 2, 2020 •

edited

Loading

amirfefer left a comment •

edited

Loading

amirfefer Oct 22, 2020 •

edited

Loading