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

[New] Updated Label screen (adapted from Tyleryandt18's Unified diary screen) #883

Merged
merged 16 commits into from
Sep 27, 2022

Conversation

sebastianbarry
Copy link
Contributor

Tyler's old PR: #870

This is the continuation of the Unified diary screen changes. View the previous PR to see last changes made.

Next steps:

I DID NOT add the new label screen directive in the old diary screen in the interest of time because it was taking me a while to figure out. Also, if we are planning on eventually getting rid of the old Diary screen and incorporating it into this new improved Label (with the Diary screen built in) screen, then it may be a waste of time trying to figure this out.

With this and my previous Commits, we should be good to ship this as-is for testing. Then...

Next steps

  • Incorporate the trip guessing percentages (car percentage, bike percentage, walk percentage) in the infinite-scroll-trip-item directive
  • Incorporate the date-picker functionality from the /root/main/diary into the /root/main/inf_scroll screen
  • Remove the Diary tab from main.html
  • Change the Infinite scroll tab's translate name from main-inf-scroll.tab to main-diary, and the icon from icon="ion-checkmark-round" to icon="ion-map"

tyleryandt18 and others added 11 commits July 7, 2022 17:20
Pulled out functionality from the InfiniteDiaryListCtrl into the new module, 'infscrolltripitem'. This new scope includes the surveyOpt, itemHt, and showDetail function for the new directive. This relates to issue e-mission#740, Unifying label and diary views.
Created a new module for a diary item directive. Pulled out the necessary functions; created the diary_list_item.html as a template for the directive.
These changes include the stylistic changes to the css layout of the unified diary screen, changes to the multi-label button layout, changes to the java script of the trip item directive, and removing the diary tab from the bottom ion-tab.

Issues -
- reading in the trip percentages; When I attempted to use the DiaryHelper.getPercentages in converting the trip object to geojson in the trip item directive, the tripgj did not have a sections attribute. I then went into services to see if I could add this, and then I went down a rabbit hole of different functions missing different attributes. I will attempt to fix this soon.
Not yet added displaying the trips only for that date
#91d4ff -> #80D0FF
pushnotify.js
- There was one instance of the old Teal, replaced with NREL Blue

main-metrics.html
- Replaced all instances of internal html hard-coded Teal with hard-coded new NREL Blue. We may want to find a new way of doing this that is not hard-coded, but for now it is Blue
main.diary.css
- Added a new .diary-map-shell to be used in trip_list_item.html
- Set width in .diary-map to be 100% so that it fits fully within the .diary-map-shell
- Changed the bottom padding of .diary-infos from 10% to 2% to display properly
- Changed the height of .diary-infos to be 50% so it can be split into 2 parts on the diary screen

style.css
- Changed the color of the gray buttons on the profile tab to be darker gray
- Changed the color of the red logout button on the profile tab to be darker red

pushnotify.js
- Changed old Teal to new NREL Blue

trip_list_item.html
- Used new diary-map-shell outside of the existing diary-map so that you can ng-click the Leaflet map
- Split the diary-infos div into 2 diary-infos divs so that we can click the top one, and the ng-click won't interfere with the lower div

main-metrics.html
- Changed all locations of the old Teal to the new NREL Blue
*THIS APPEARS TO BE HARDCODED, MAY WANT TO FIX THIS LATER*
main.html
- Uncommented the diary button
- Replaced the icon and title from the unified diary button, back to the label button
- Remove any instance of the font-family: 'Lato'
- In main.diary.css, change the font size back to 0.8em !important from 12px
- Remove any instance of colors which are not in the NREL Official color pallette, replace with NREL Official colors
- Reverted "control-icon-button" changes, and created new "Diary-button" style. Change all instances of "control-icon-button" in Diary javascript to "diary-button"
- Reverted all changes made to the ionic.css files, reproduced these changes as css style overrides in main.diary.css
-
@sebastianbarry
Copy link
Contributor Author

From the old PR containing Tyler's code review changes (#870):

Here is my PR with the current response to Tyler's code review: #883

There are still a handfull of things to go over; any conversation that doesn't end in Confirmed this change as complete still needs to be addressed.

sebastianbarry and others added 3 commits September 23, 2022 12:26
infinite_scroll_trip_item.js
- Removed the event listener for Leaflet Map Resize, whcih is a workaround to fix android map tiles not fully loading, which the invalidateSize() workaround makes it work again
This code is copied from www/diary/detail.js and is not needed in this file.
style.css
- Removed start-time-tag-inf-scroll as it is not being used anywhere in the code
- Removed stop-time-tag-inf-scroll as it is not being used anywhere in the code
@sebastianbarry
Copy link
Contributor Author

I resolved another display issue:
Before, the top of the label screen had text which was overwriting it's div, and going into the rest of the page:
Screen Shot 2022-09-26 at 10 25 09 AM

I compared it to our current label screen on the main branch:
Screen Shot 2022-09-26 at 10 51 41 AM

I then found out the checkbox button was using the wrong style, so I corrected the style and it is now displaying properly again:
Screen Shot 2022-09-26 at 10 55 21 AM

infinite_scroll_list.html
- switched the checkmark button style class from "diary-button" to "control-icon-button", which is a smaller sized button and allows the text to display on it's line
@sebastianbarry
Copy link
Contributor Author

@shankari can you review my code for the last 3 commits?

@sebastianbarry sebastianbarry changed the title [New] Unified diary screen (adapted from Tyleryandt18's Unified diary screen) [New] Updated Label screen (adapted from Tyleryandt18's Unified diary screen) Sep 26, 2022
@shankari
Copy link
Contributor

shankari commented Sep 27, 2022

We've spent multiple review sessions on this, so I'm going to merge this and push out a new release to Android + TestFlight for testing. @sebastianbarry please stay ready to make changes if we find any issues while testing.

@shankari shankari merged commit 6db4364 into e-mission:master Sep 27, 2022
@shankari
Copy link
Contributor

shankari commented Sep 27, 2022

@sebastianbarry I found another issue while merging.
The modified www/templates/survey/multilabel/multi-label-ui.html does not have the ng-attr-id. Are we actually using that id anywhere? If not, we are right to remove it.

Actually, nvm, it does exist.

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.

3 participants