A modal windows has to be linked to the button as shown in issue #33. Is there any standard modal plugin which was used before for fedora-hubs? Modal window has been already created as shown in issue #33 by using a custom modal plugin.

I'm a little confuse, is the comment 1 related to the PR ?

The PR seems to be about a footer for the widget's panel while the comment speaks about a modal/panel module.

I'm a little confuse, is the comment 1 related to the PR ?
The PR seems to be about a footer for the widget's panel while the comment speaks about a modal/panel module.

In the footer, there is a button for request a new meeting which is linked to a modal window. I didn't include the modal window yet since i don't know whether there is a standard modal plugin for fedora-hubs. So i thought before jump into the next part, it is better to ask what plugin I should use for implementing modal window.

is the indentation proper here? It looks out of place. same with footer initialization above

Indentation required here.

This block should be indented,

Something weird happened in the patch file. Indentation errors are shown only in the patch file, not in the source files. I tested it before pushing to the repo. However i updated the pull request again.

I suspect your editor might be wrongly configured and you may be mixing tabs and spaces (we use 4 spaces indentation), so what looks the same is in fact not.

I suspect your editor might be wrongly configured and you may be mixing tabs and spaces (we use 4 spaces indentation), so what looks the same is in fact not.

The patch file is not updated yet. May i create a new pull request?

Just push your changes to the same branch, the pull-request will be automatically updated.

Looking at the diff, here on pagure seems to look fine now, does it not for you?

diffs are fine for me too.

There is a trailing slash here that we should remove now or after the merge

@kjtdimuthu

Ok I tested the PR locally and it seems to be doing the job. The code looks fine but I am wondering about merging this PR manually as the 4 commits are looking odd:

• Of the 4 commits, 3 have the same commit message, are really all 3 commits doing the same thing? Beware that the commit message actually reflects what is changed
• These 3 commits message are too long, cf https://git-scm.com/book/ch5-2.html for the long version, but basically, the commit message should be a first line with a short description (50 chars or less ideally, but I do go up to 80 and even a little beyond if that helps) then an empty line then a more detailed description of the change if needed

Do you think you could squash all the commits into one and adjust the commit message? You will need to push --force to update the PR after you squashed them but it would make the history a little clearer.

Feel free to ping me on IRC if you have any problem squashing, and do not push if you have any doubts about what you did :)

@pingou
Previous commits were squashed :)

Hi @kjtdimuthu,

Thanks for squashing the commits, however, the commit message is still too long, did you check the link I sent above?

I'm going to merge #145 which will likely conflict with this PR, so we'll have to fix these conflicts before we can merge it :)

I merged conflicts and changed the commit message :)

Ok, let's remove the style and the id for the moment (the style needs to be fixed in the css if at all) and the id is too generic for now, we'll bring it back when the button actually does something :)