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.

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

146 navigation webapp #147

Merged
merged 13 commits into from
Jan 29, 2021
Merged

146 navigation webapp #147

merged 13 commits into from
Jan 29, 2021

Conversation

rheaSPK
Copy link
Collaborator

@rheaSPK rheaSPK commented Jan 22, 2021

Closes #146
image

Since the dashboard page has it's own footer and doesn't provide independent routes for chatting and dashboard, the current dashboard page looks like this:
image

Note, that chat and dashboard have the exact same routes, that's why the footer shows both be clicked. I expect that the dashboard will be redesigned eventually, that's why I don't see an issue with that right now.
Also there is no real settings page, I've put edit_user_path for the current user instead. If there's a settings page eventually, this has to be replaced.
Since we have no history page at all, this link isn't linking anywhere.

@LinqLover
Copy link
Contributor

Woah, looks awesome! I don't think we should show two footers at the same page, this would be very unintuitive.

Another hint: Could you use bootstrap-select for the status choice? It's an extenal library but quite simple to use and would look much more better than the current plain HTML solution IMHO. What do you think? :)

@rheaSPK
Copy link
Collaborator Author

rheaSPK commented Jan 23, 2021

@LinqLover

Woah, looks awesome! I don't think we should show two footers at the same page, this would be very unintuitive.

This is indeed unintuitive, but the whole dashboard has to be redesigned anyway. Right now there is just an example text. I'll ask if someone is already doing this and ask them if they could integrate my solution to their dashboard. The ticket just said it wanted to have a header and a footer, so I guess it editing the dashboard would be out of scope

Another hint: Could you use bootstrap-select for the status choice? It's an extenal library but quite simple to use and would look much more better than the current plain HTML solution IMHO. What do you think? :)

AFIK I don't use a select dropdown menu in this design because it wasn't in the scoop adding a status choice or anything, so there's no need for bootstrap select. Or what do you mean?

# Conflicts:
#	app/assets/stylesheets/mobile.scss
#	app/views/layouts/_navbar.html.erb
#	spec/features/navbar_spec.rb
@LinqLover
Copy link
Contributor

Another hint: Could you use bootstrap-select for the status choice? It's an extenal library but quite simple to use and would look much more better than the current plain HTML solution IMHO. What do you think? :)

AFIK I don't use a select dropdown menu in this design because it wasn't in the scoop adding a status choice or anything, so there's no need for bootstrap select. Or what do you mean?

Because the dashboard will be redesigned anyway, we leave it as-is for the moment.

Copy link
Contributor

@LinqLover LinqLover left a comment

Choose a reason for hiding this comment

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

@rheaSPK Only a few formal things, UX is fine :-)

app/assets/stylesheets/home_mobile.scss Show resolved Hide resolved
spec/features/navbar_spec.rb Show resolved Hide resolved
ubuntu-bionic-18.04-cloudimg-console.log Show resolved Hide resolved
@LinqLover
Copy link
Contributor

@rheaSPK The ubuntu image is still in the repository. Apart from this, looks perfect now :-)

Copy link
Contributor

@LinqLover LinqLover left a comment

Choose a reason for hiding this comment

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

provided that you will remove the ubuntu log file before merging :)

@rheaSPK rheaSPK merged commit 4272e15 into dev Jan 29, 2021
@rheaSPK rheaSPK deleted the 146_navigation_webapp branch January 29, 2021 16:33
@frcroth
Copy link
Contributor

frcroth commented Jan 29, 2021

@rheaSPK Thank you for your effort! I think the navigation can still be improved.

  • What does the 4th button (the fa-history button) do / what is planned here?
  • You can remove the dashboard navbar, as activities and proposals are not part of a scenario right now. This would remove the need for a second navbar

@LinqLover
Copy link
Contributor

@frcroth Interesting ideas, could you create a new issue for them?

@frcroth
Copy link
Contributor

frcroth commented Jan 29, 2021

OK #160

LinqLover added a commit that referenced this pull request Jan 29, 2021
Commited by accidented via #147. @rheaSPK FYI: Because you had added the file to the .gitignore first, you should have manually `git add`ed the file after removing it in your working copy ... :-)
rheaSPK pushed a commit that referenced this pull request Jan 29, 2021
Commited by accidented via #147. @rheaSPK FYI: Because you had added the file to the .gitignore first, you should have manually `git add`ed the file after removing it in your working copy ... :-)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation in the web-app
3 participants