Reporting or contributing for the Bootstrap 4 upgradation

Souravirus commented 5 years ago

Hi everyone,

We are working on the upgrade to Bootstrap 4 of the Public Lab website(publiclab.org) in this PR. For some days from now, we will be keeping the unstable branch updated with the Bootstrap 4 commits. So, if anyone sees anything odd at https://unstable.publiclab.org/, please report to https://github.com/publiclab/plots2/pull/3937.

Also, if anyone wants to correct the error or the existing errors, please push to the Souravirus:bootstrap4 branch.

https://github.com/Souravirus/plots2/tree/bootstrap4

Thank you!!

grvsachdeva commented 5 years ago

@publiclab/plots2-reviewers @publiclab/reviewers

PritiShaw commented 5 years ago

Can i work on this ??

grvsachdeva commented 5 years ago

yes @PritiShaw :+1:

jywarren commented 5 years ago

Hi!! Ok, going through a lot of small and big things here, but just want to say 🎉 this is AMAZING @Souravirus !!!

[x] If we could get a little more space between the search bar and the next menu, that'd be great:

[x] I think the "show password" button that @CleverFool77 made is getting wrapped, although it still works!

[x] I think the "profile pic" is not floating right in this:

[x] space needed between "or" and "log in" here:

More soon!!! Great work, again!

jywarren commented 5 years ago

[x] I see this in a few places where a btn group is kind of not smoothly grouped, like on the dashboard:

[x] And after pressing the "Wiki" tab, but also the "New page" button:

[x] The "subscribe" button doesn't seem to be working?

[x] the tags "cog" menu is off the side of the screen:

[x] the geocoding modal is not full-width:

[x] Adding a new tag doesn't clear the input, and the new tag is not in a colored badge element:

[x] Some weirdness with the boxes around the new comment box:

[x] the "preview comment" button doesn't seem to work?

[x] the node tool caret icon is not appearing:

[x] clicking the "like" button doesn't seem to do anything
[ ] follow buttons on the /tags page don't work:

That's a lot for now! Thanks so much for your hard work on this project, Sourav!!!!!

jywarren commented 5 years ago

Feel free to check/resolve mine as they get solved. Thanks again!!!

Souravirus commented 5 years ago

Hi @PritiShaw if you want to contribute to these issues you can just contribute to this PR. Thank you!!

Souravirus commented 5 years ago

Hi @jywarren, that show password button is getting wrapped because the form-control-feedback class is outdated in Bootstrap 4. So, should just append it to the input box?

jywarren commented 5 years ago

Ah, yes, i guess so. Thanks!

On Mon, Apr 8, 2019 at 11:08 AM Sourav Sahoo notifications@github.com wrote:

Hi @PritiShaw https://github.com/PritiShaw if you want to contribute to these issues you can just contribute to this PR. Thank you!!

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/5182#issuecomment-480872159, or mute the thread https://github.com/notifications/unsubscribe-auth/AABfJ885-TZmzXsCmLLmCC6ly3MNObpcks5ve1tKgaJpZM4b8_JR .

sashadev-sky commented 5 years ago

This is not specific to any version of Bootstrap, but please note in PR #5372 we found and resolved a bug with Bootstrap that is yet to be fixed even in V.4. It is that checkboxes do not vertically align with their text for certain OS's. There is now a stylesheet in plots2 specifically to explain and override this bug: https://github.com/publiclab/plots2/blob/master/app/assets/stylesheets/btsp_checkbox_override.css.scss

Those classes are currently not being used anywhere else in the repo besides rich.html.erb (the editor), but please take these overrides into consideration if you do use them. Note they are also on a very high level of specificity.

sashadev-sky commented 5 years ago

Also another thing: the styling Bootstrap applies to input[type=file] does not prevent cross-browser inconsistencies with the alignment of input[type=file] buttons and the text printed next to them. I added another css file to address that (please see PR #5424 and the issue it references)

Souravirus commented 5 years ago

Hi @sashadev-sky, can we include these after the upgrade or someone can take up this task and put the classes where they are required and I can merge them? What do you think about this?

sashadev-sky commented 5 years ago

@Souravirus they aren't a big deal so can be included at any point. One of them is already merged and i'm building on it in PR #5421 to apply to every checkbox because I realized all of the check boxes on the site are slightly unaligned for certain users

Souravirus commented 5 years ago

Hi @jywarren, the New page button is appearing fine in my browser. Here is the screenshot

Souravirus commented 5 years ago

@jywarren, just wanted to say the dropdown which is moving out of the screen can be accessed with the slider at the bottom. Also, it is done by default by Bootstrap. Talking about the map modal, it is also the default size of the Bootstrap modal. So, should we leave them as it is?

Souravirus commented 5 years ago

I have made the map modal to a larger size. Here is the screenshot. Screenshot from 2019-04-14 16-58-23

Souravirus commented 5 years ago

Hi @jywarren, the subscribe button is even not working in the original public lab website in the tags page. So, there is a problem with it already in the main repository. So, we can open another issue for that. Also, with that, all the issues are solved. Please see to this. Thank you!!

Souravirus commented 5 years ago

Hi @jywarren, now every bug is solved except the one about the follow button on the tag page which was already present and was not due to bootstrap 4. I have also made the dropdown alignment correct. Here is the screenshot of it.

Please see to this.

jywarren commented 5 years ago

Oh this is super. Let's do one more pass and then... go for it? @sagarpreet-chadha @gauravano @SidharthBansal @cesswairimu @stefannibrasil @milaaraujo @ViditChitkara can you try this out and highlight any serious issues?

We will surely have a few days where things get broken a lot, but we can minimize it with one last final check!!! 👀

jywarren commented 5 years ago

@Souravirus will you be pretty available next week if we merged and published this, to help with fixes? Thank you SO MUCH for all of this.

Souravirus commented 5 years ago

Yeah I will be available the next week. Thanks!!

stefannibrasil commented 5 years ago

Is there a PR with the changes? I could take a look, for sure! :v:

stefannibrasil commented 5 years ago

This doc was really helpful when was doing the same on MapKnitter -> https://getbootstrap.com/docs/4.0/migration/

jywarren commented 5 years ago

Yes indeed! https://github.com/publiclab/plots2/pull/3937/ Thanks!

On Fri, Apr 19, 2019 at 4:06 PM Stefanni notifications@github.com wrote:

This doc was really helpful when was doing the same on MapKnitter -> https://getbootstrap.com/docs/4.0/migration/

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/5182#issuecomment-485001366, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAF6J2KDDXAWTHDXUWJT53PRIQ2HANCNFSM4G7T6JIQ .

sashadev-sky commented 5 years ago

This will have no changes on Bootstrap upgrades at all but I wanted to post here that I made a PR to upgrade sass-rails to sassc-rails (it really doesn't change anything at all except a few file names so it is safe to merge in my opinion!) . Please see here: #5549

Souravirus commented 5 years ago

Hi @stefannibrasil, you can see the changes in the unstable branch.. Thank you !!

stefannibrasil commented 5 years ago

HI all, I did a walk-through on the unstable branch and the only difference that I noted was a space missing after the Log in button in the Log in modal. Other than that, looks good to me :shipit:

Souravirus commented 5 years ago

Hi @stefannibrasil, I am really sorry that the unstable branch was currently not holding the bootstrap 4 changes as there were other commits pushed to it. So, can you please see the changes again sometime later. Thank you!!

Souravirus commented 5 years ago

Ok I have pushed the changes. Please see it again after some time. Thank you!!

stefannibrasil commented 5 years ago

oh, okay :v:

The front page is looking weird. Maybe someone pushed other commits again? The carousel is not working and all of the static content (social media icons, questions button, etc.) are not showing.

Some spaces to be added:

between the nodes and the pagination buttons (front page);
after node types buttons on /search page;
between nodes results ( the read more button is attached to the other node's image);

General notes

Any misalignments seen in radio buttons or checkboxes can be fixed using the classes in btsp_checkbox_override.css.scss. Note these misalignments were there before the update and the update has not fixed them
The footer is different when you navigate between the pages. For example, in the Dashboard, I see a different footer than the one being shown at the front page;
Also in the Dashboard, when I clicked on 'Share your work', the dropdown is floating to the right, not beneath it;
When adding a question, in the button group, the Markdown is not at the same line as the others;
In the highlighted note that appears in the Dashboard, the text 'Node title by user1 from Public Lab Blog` is not aligned;
The 'enter tag' placeholder is not possible to read here, for example, https://unstable.publiclab.org/getting-started (maybe removing the caret solves this, since it's already visible that it's a button?)
The button group here is not aligned https://unstable.publiclab.org/wiki/plots-staff
Align the button hat the /subscriptions page;
I think these buttons for Questions and Research notes are too big https://unstable.publiclab.org/wiki/organizers
Just the 'logout from all devices' option is showing up, not sure if this is intentional;

I think that was all, good job! :tada:

grvsachdeva commented 5 years ago

[x]
[x]
[x]

@Souravirus can I commit directly on your PR to resolve some bugs? Thanks!

grvsachdeva commented 5 years ago

@Souravirus I have opened a PR against your bootstrap4 branch - https://github.com/Souravirus/plots2/pull/1

grvsachdeva commented 5 years ago

Moving to #5612

grvsachdeva commented 5 years ago

Thanks everyone!!

publiclab / plots2

Reporting or contributing for the Bootstrap 4 upgradation #5182

Some spaces to be added:

General notes