-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[feat] result types: add weather result answerer and template #4663
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
Conversation
7b5879b to
94ac907
Compare
|
FYI: last force push was just a rebase on master branch. |
|
I pushed a WIP commit .. but I haven't yet finished my work .. I set this PR to DRAFT and will inform you, when I have finished my work .. he must have patience, the young jedi ;-) |
88ee545 to
7f08749
Compare
c7c8bbf to
181166c
Compare
|
Haven't yet finished my work, but if you want to have a look at current state .. I added types, i18n/l10n, symbols (icons) and unit conversions (based on what we had in the converter plugin) .. and when selecting search language To see forecast, click on the table (or weather symbol) .. |
8fef6cd to
c64636f
Compare
Bnyro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great already, thanks for your work!
I've left some comments / suggestions / questions.
c64636f to
91cbe67
Compare
91cbe67 to
32d136d
Compare
I have adopted some changes, see diff .. and I have left a comment with the others. Switched PR from DRAFT to "ready for review" .. |
| margin: 0; | ||
| padding: 0.5rem 1rem; | ||
| background-color: var(--color-header-background); | ||
| .rounded-corners-tiny; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, we can fix it by replacing the <div class="summary"> with <span class="summary">, that's the simplest fix 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your first suggestion "list-style: none" works better .. for now .. the <summary> element is the simplest solution... it's a bit impractical because you can only expand/collapse it in the header. We should perhaps turn it into a horizontal (left/right) slider in a future PR.
|
Looks awesome to me after all, thanks for all the efforts you've put into this! I already thought about porting the other weather engines to using the new API but I think that would be better suited for a new PR, so we should merge this here first. |
The types necessary for weather information such as GeoLocation, DateTime, Temperature,Pressure, WindSpeed, RelativeHumidity, Compass (wind direction) and symbols for the weather have been implemented. There are unit conversions and translations for weather property labels. Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
32d136d to
ffaf7ad
Compare
|
There it is, finally 🚀 |
Author Notes - wttr.in provides 8 hourly time forecasts per day, I assumed that they're always describing the weather for 3 hours each, starting at 1 o'clock in the morning related: - searxng#4663 - searxng#4885
What does this PR do?
Why is this change important?
How to test this PR locally?
!om BerlinAuthor's checklist
Related issues
superseeds/closes #3256
related: #4542, #1800, #1352