Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hotfix/#10 sidebar ie issue #78

Closed
wants to merge 30 commits into from
Closed

Hotfix/#10 sidebar ie issue #78

wants to merge 30 commits into from

Conversation

vickyokrm
Copy link

This fixes the issue#10 with respect to the side bar display issue on internet explorer

vickyokrm and others added 19 commits August 22, 2017 14:54
@vickyokrm
feature/#62-refresh-send-icon
213e907
@vickyokrm
feature/#62-refresh-send-icon
4f9295a
@snoozan @geekgonecrazy
Fixed function closure syntax allowing validation emails to be sent.
3100b6f
@snoozan @geekgonecrazy
[FIX] Fixed function closure syntax allowing validation emails to be …
d8d386b 
…sent.
@geekgonecrazy
Bump version 0.58.2
a1da0bd
@geekgonecrazy
Merge branch 'release/0.58.2' of https://github.com/RocketChat/Rocket…
8f8f2bb 
….Chat into release/0.58.2
@geekgonecrazy
Add history
cbaf9c1
@rodrigok
Merge pull request RocketChat#7841 from RocketChat/release/0.58.2
b613b6f 
Release 0.58.2
@geekgonecrazy
Fix issue caused by merge
988103d
@ruKurz
Merge pull request #83 from mrsimpson/feature/#62-refresh-send-icon
cc4bf74 
Feature/#62 refresh send icon
@mrsimpson
Make fix for input selection more robust (#84)
15a18a8 
* use Meteor's internal selector instead of a global jQuery

* Make the focus input event a bit more robust

* Undo accidental cut

* code style
@mrsimpson
Merge branch 'master' into develop
d5a3eb2
smarti#87 add support area to smarti
3ac051d
@rodrigok
Deps update
a494cf5
@rodrigok
Bump version to 0.58.3
bd33117
@mrsimpson
Merge pull request #62 from assistify/smarti#87_add_support_area_to_s…
93a6baf 
…marti_call

Add support-area to smarti
@mrsimpson
Merge branch 'hotfix/#63-global-announcement' into develop
dfde313
disable smarti load if necessary parameters are not set
ca8e4b7
@vickyokrm
fix
6d096f4
@ghost ghost assigned vickyokrm Oct 5, 2017
@ghost ghost added the progress:working label Oct 5, 2017
@mrsimpson mrsimpson self-requested a review October 5, 2017 21:29
mrsimpson
mrsimpson requested changes Oct 5, 2017
View reviewed changes
Copy link
Member

@mrsimpson mrsimpson left a comment

Choose a reason for hiding this comment

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

@vickyokrm seems you branched off the wrong branch. It should have been master :(
Let's fix that together tomorrow

mrsimpson
mrsimpson reviewed Oct 5, 2017
View reviewed changes
Copy link
Member

@mrsimpson mrsimpson left a comment

Choose a reason for hiding this comment

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

hmpf - don't understand why the global announcement is not part of master... let's see that tomorrow

packages/rocketchat-theme/client/imports/base.css
@@ -3035,7 +3035,7 @@ body:not(.is-cordova) {

.flex-tab-container {
display: flex;
flex: 0 0 41px;
/*flex: 0 0 41px;*/
Copy link
Member

Choose a reason for hiding this comment

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

@vickyokrm this is the actual fix, right? Have you tested other browsers?

Copy link
Author

@vickyokrm vickyokrm Oct 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@mrsimpson Tested in the chrome and IE.

Copy link
Member

@mrsimpson mrsimpson Oct 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@vickyokrm what about using the -ms-flex and -webkit-flex (see https://www.w3schools.com/cssref/css3_pr_flex.asp)

Copy link
Author

Choose a reason for hiding this comment

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

@mrsimpson, machi, -ms-flex was supported until IE 10. From IE 11, the flex property should work without any issues. Whereas in our case, although the flex-basis property was fixed to 41px, webkit recomputes the flex-basis value depending on the width of the child flex items, while IE fails to do that which leads to no room for the dynamic overview screen. An another approach to make this work in IE will be by setting the 'flex-basis' to 'auto'.

Copy link
Member

Choose a reason for hiding this comment

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

@vickyokrm flex: 0 0 auto is equal to "flex: none".
Anyway, I'll just merge your PR, please discuss the flexing with the RC core team (at RocketChat#8204) so that we can understand why they wanted to have a basis of 41px - it surely has got some reason

Copy link
Author

Choose a reason for hiding this comment

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

@mrsimpson The fixes to handle the multiple duplicate scroll bar issue in the side nav bar is resolved. Seeking your assistance to review it.

rodrigok and others added 6 commits October 5, 2017 21:30
@rodrigok
[FIX] Meteor accounts issue - function which caused users to be logge…
3a90df3 
…d out shortly after logging in
@rodrigok
Merge pull request RocketChat#8389 from RocketChat/fix-snap-npm-dep
3e69c89 
[FIX] Add needed dependency for snaps
@rodrigok
Merge pull request RocketChat#8390 from RocketChat/fixes-for-slack-im…
c51011e 
…port

[FIX] Slack import failing and not being able to be restarted
# Conflicts:
#	packages/rocketchat-i18n/i18n/en.i18n.json
@rodrigok
Merge pull request RocketChat#8408 from RocketChat/rest-api-improve-j…
cf1eef4 
…son-query-helper

[FIX] Duplicate code in rest api letting in a few bugs with the rest api
# Conflicts:
#	packages/rocketchat-api/server/api.js
#	packages/rocketchat-api/server/v1/channels.js
#	packages/rocketchat-api/server/v1/groups.js
#	packages/rocketchat-api/server/v1/im.js
@rodrigok
Bump version to 0.58.4
60fa18c
@mrsimpson
fixes #10 - tested in chrome and IE on windows
1be4d95
@ghost ghost assigned mrsimpson Oct 6, 2017
@mrsimpson
Copy link
Member

@vickyokrm the sidebar flexes out now, but there are two scrollbars - which looks ridiculus.
Furthermore, as we tested it, Smarti widget didn't load. Can you please check that?

@mrsimpson mrsimpson mentioned this pull request Oct 13, 2017
Closed
@mrsimpson
Copy link
Member

please create from and PR towards master

@mrsimpson mrsimpson closed this Oct 25, 2017
@ghost ghost removed the progress:working label Oct 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrsimpson mrsimpson mrsimpson requested changes

Assignees

@mrsimpson mrsimpson

@vickyokrm vickyokrm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vickyokrm @mrsimpson @snoozan @geekgonecrazy @rodrigok @ruKurz