-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: Mobile friendly #86
Conversation
Hey thanks a lot for taking the time, I can already see that it's better than what it used to be. But if we ship this, then I would like a few tweaks to get it right the first time. See my comments and proposed changes in the rough sketch below. For the map, I think the component will have to be split into two parts but the data must only be fetched once. Or maybe a better alternative is to just hide it on mobile and show the text table. It's difficult to find meaningful information when the map is so small on mobile, so I believe hiding it is the better alternative. |
Yeah, I like your tweaks! When viewing in test on my screens it didn't
look quite that bad - and I didn't try the demo mode. What screen size
approximately were you trying there? And, is there an easy way to turn on
the demo banner when I'm just viewing the frontend with "npm run dev". Ooh
- is there an easy way to feed the frontend with data when doing npm run
dev? I was also just looking at blank things so I probably missed some bad
text flow.
I'll try to look at this this weekend, and if there's not a good way to
send in data then I'll hack something in so my work is better.
The first take was intended to be a light touch, and very size-flexible. I
think these will require a heavier hand, maybe moving some parts of the
templates a little and maybe duplicating some parts but having the
duplicate display/not display based on screen width.
The map looks really cool - you're right it definitely needs to be two
parts or just have the map disappear/hide. Fetching data only once is no
problem, it should be possible to shift things around without changing how
the underlying fetch/display code works.
…On Mon, Apr 8, 2024, 01:47 Rehan van der Merwe ***@***.***> wrote:
Hey thanks a lot for taking the time, I can already see that it's better
than what it used to be. But if we ship this, then I would like a few
tweaks to get it right the first time. See my comments and proposed changes
in the rough sketch below.
image.png (view on web)
<https://github.com/rehanvdm/serverless-website-analytics/assets/22810856/9e615e8b-9d34-4e13-852a-654dd1c8a45e>
For the map, I think the component will have to be split into two parts
but the data must only be fetched once. Or maybe a better alternative is to
just hide it on mobile and show the text table. It's difficult to find
meaningful information when the map is so small on mobile, so I believe
hiding it is the better alternative.
—
Reply to this email directly, view it on GitHub
<#86 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALGLJKC3SYUXOVWDSNWTNDY4I4Q3AVCNFSM6AAAAABF3CFHIOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRHE4DCNZRGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I found the contributing doc now with the backend instructions, btw. |
I was just using this feature of the browsers and selecting a few, not sure which size exactly, but this gives an idea
If you get your backend running locally, small mention of it here then there is a flag that you set here in the test config along your other parameters. Sorry, there are no frontend tests or easy ways to run and simulate large numbers as on the demo page. My idea was to tackle that problem if a lot of people struggle with getting it and tests running to revisit and improve it to make it easier. Let me know if you got |
Shrank container min-width to allow more flexibility with content shrinking. Now set to 160px, but including padding the minimum page width before the browser just starts shrinking content is about 220px. Allowed cards to grow to fit space, so now if one of the 4 little data points drops to the next line it'll show as centered and look nice. Also increased card min width and matched units to the font unit - so, essentially, the data points can show up to 8 or 9 large characters across before content will overflow. Imported the Menu icon from vue-material-design so it can be used for the page view/event selector. Added names to page and event in the router so the names can be displayed. Modified the demo banner to work better at small screen widths. Components will not encroach on the close button. The close button and right side of the box are scooted away from the right wall a little. The github links can now flex to a top line and push the text down below when there's not enough room for both of them on one line. The bar charts in view and events have overflow and width set on them as constraints. I'm honestly not sure why that makes them perform more consistently at smaller sizes in Firefox. The 300 px width is not a magic number - the important part is that it's smaller than expected chart width at screen widths larger than 700 px. This random fix only makes a difference at screen widths larger than 700 px... Tested in Firefox and Chromium. Site selector and date selector are now in a flexbox just by themselves, so when the screen gets too small for both then they can split across two lines and take up the full line. Looks nice I think. There's a new flexbox for the settings buttons and refresh button. Views/Event content setting is in there (when visible), refresh button, and theme settings (cog) button is in there. It's separate from the site/date selector so the site/date selectors flow nicer, and to allow this one to flow up in narrow mode. 700 pixels wide is the point at which the display switches mode. The normal left side views/events bar becomes invisible when the screen is narrower than 700 px now. The views/event "Content" selector hamburger button becomes visible when the screen is narrower than 700 px. This has an el-drawer implementing the popout window. The worst part about this is that this code is similar across two files now, instead of just being implemented in one place: views/stats/index.vue and vies/app.vue The settings button flexbox (content, refresh, cog button) moves up a line, above the site/date selector, and takes up the whole width, when screen is narrower than 700px. The map now pushes down below the location stats listing so they can both take the full width. I think the map is a really cool feature, and easy enough to scroll past if you don't want to see it.
I think it's in-line with the suggested view now! My commit 8ff3f1e has a long description attached with all the details about what changed. I kept the map at smaller sizes - I think the map is really cool... It does move down though. I ended up writing a little service that just responded to API requests with canned responses copied out of the demo site - that was a simpler solution than figuring out how to do it right I think ;-) |
This looks great! Thanks for all the effort. I see a few regressions on the bigger view that I tried to capture in the image comparison below, let me know if something isn't clear or if you have questions/suggestions. ElementPlus has utilities to show/hide components depending on screen size https://element-plus.org/en-US/component/layout.html#utility-classes-for-hiding-elements, I tried to stick to using the framework as much as possible to this point. But I don't mind, I think it is fine like you have it, not too complicated. Learned a few new things:
|
I rejiggered how this padding layout worked... I believe the goal with the original large screen layout was to have 15px on top, bottom, and sides, and good space between elements sideways. With the flexbox layout, and the possibility of multiple rows of stats, just putting 15px on the cards makes 30px between cards vertically. I think that looks slightly less balanced than intended. So - now the gap (flexbox thing) manages space between elements at only 15px, and the padding is applied to the outer element (totals) to handle the top/bottom/sides padding.
Vertically centers the page/event button, refresh, and gear buttons, and slightly changes how separator is calculated to reflect local size instead of root (rem to em) so it'll hopefully stay centered better in future changes. Also moves the gear right to the edge... Not sure if you'll like this @rehanvdm, when I was aligning the hamburger button I realized there's no reason it shouldn't go all the way left to the edge, there's padding to keep it away from the screen edge... Then I realized the gear should work the same on the other side.
Also - make the page break on the map simpler by just changing to one flex column, instead of switching layout mode to flex.
To do this without negatively affecting responsiveness, I added an additional media break at 900px. When the screen is less wide than 900px (max-width 899px) then the site and date selector are allowed to take up the full width and flex their sizes more, so they can take a good view size regardless of width.
Sounds good. The scrollbars on those elements - sorry about that. There was something weird happening in my browser off-and-on and it was kinda fixing it and not causing a problem. I think the interaction between plotly resizing and browser is just flaky when testing the way I am. I am not going to make the hamburger, page/event, mobile view button bigger - because I'm not sure how much bigger to make it. Right now it's the same size as the refresh and gear buttons which... I think is nice. The reason time on page column needed to be slightly wider to fit the large view is because the views column needed to be slightly wider as the screen narrowed even a little... I made time on page slightly wider to just get it done now... But I think the better solution might be to use just a regular HTML table, or to use just one grid view for the whole table instead of a grid view for the rows then a grid view for the columns for each row. I think both those solutions would allow the browser to determine the correct column widths on the fly, regardless of screen width, instead of statically determined at build time. I will mess around with that and see if it yields something more "responsive". The built in elementplus utilities are interesting... The necessary CSS for that is not included in the site right now, we'd have to add I realized I don't have to change from grid to flex on screen width change, I can just change the number of columns :-) So I did that now. Simpler is better :-) To make the site and date selectors be the original size on large screen, while still allowing them to flow to reasonable sizes/drop to another line, I added in another media size break at 900/899 px, where they're allowed to take up full width below that size, and they're allowed to wrap down a line. Sorry to throw in two bugfixes in the middle of these commits, too, but I noticed JS errors in the console when I was working on this, and fixed a typo in TableData.vue (v-lese instead of v-else), and the way the tooltip content was calculated in there (had to convert the number to a string for output). |
Thanks! This looks great. Thanks for picking up those typos, thought You mentioned that you still want to look at using a table view instead? Or is this ready to be merged? I am happy with the grid view for now. I can't remember why I changed to grid instead of table, I think it might have been because I needed to hide the |
Pulling the trigger. Thanks for this contribution! 🍻 |
🎉 This PR is included in version 1.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Yay! Sorry about lack of response. I'll experiment with tables somewhere
else and let you know if something nice comes out.
…On Sat, May 4, 2024, 05:44 github-actions[bot] ***@***.***> wrote:
🎉 This PR is included in version 1.10.0 🎉
The release is available on:
- npm package ***@***.*** dist-tag)
<https://www.npmjs.com/package/serverless-website-analytics/v/1.10.0>
- GitHub release
<https://github.com/rehanvdm/serverless-website-analytics/releases/tag/v1.10.0>
Your *semantic-release
<https://github.com/semantic-release/semantic-release>* bot 📦🚀
—
Reply to this email directly, view it on GitHub
<#86 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALGLJN23V5IYSPQPK73W5TZAS3YPAVCNFSM6AAAAABF3CFHIOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJUGEYTKOJYHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This gets the page closer to mobile friendly. With these changes, the site works significantly better on smaller screens. A remaining issue is the size of the map. Ref issue #15
Closes #15