Feature/search after before on #1574

dmitrysamuylovpharo · 2018-08-15T20:47:28Z

When filling in a section please remove the help text and the above text.

Summary

This is the UI handling for the feature added in mattermost/mattermost#9219 to be able to use additional flags (on: before: after:) in search to filter by a date or a date range. This implementation adds a new type of suggestion presentation showing a date picker letting the user find the date they are looking for easier

Ticket Link

https://mattermost.atlassian.net/browse/MM-11687

Checklist

Ran make check-style to check for style errors (required for all pull requests)
Ran make test to ensure unit and component tests passed
Has server changes (Feature/search after before on mattermost#9219)
Has UI changes
Includes text changes and localization file ([.../i18n/en.json]

…splay the suggestion in a list or date type of a suggestion conditionally based on the provider that triggers handling for the current flag, added conditional triggering of the blur even on the suggestion box (for date presentation we don't want to trigger blur as soon as the suggestion box input loses focus because the daypicker suggestion may require multiple clicks to get to the needed selection, particularly when changing months before selecting a day)

michaelgamble · 2018-08-16T14:13:08Z

Just took a quick look, noticing some issues:

first click date selection works inconsistently (check video: https://drive.google.com/open?id=1UDr0HT8xuP5lPr_SrqeZn47faqQZ5XZM)
navigating between months forces dialogue to disappear
color choices are too light / off brand (@asaadmahmood we should probably provide some color recommendations)
padding/margins around the component are inconsistent
strange extra whitespace on the right side of the calendar (thinking maybe its matching the component height and then left aligning - if thats the case our width should match component width)
we should consider setting a min or possibly fixed width / height

dmitrysamuylovpharo · 2018-08-17T13:24:48Z

@michaelgamble @wiersgallak this behavior is indicative of the initial implementation and not the latest code, can you guys get the test server re-created with the latest code? all of these issues were fixed already and you'll be able to take a look at @asaadmahmood 's styling updates as well

esethna · 2018-08-28T19:50:22Z

Discussed with @dmitrysamuylovpharo, (2) and by extension (1) are out of scope for this PR as it is how autocomplete works now in all parts of the app. Will create a ticket to address this separately

esethna · 2018-08-28T19:51:44Z

Green light from QA, moving to dev review

dmitrysamuylovpharo · 2018-08-28T20:02:01Z

@hmhealey there's one outstanding item here that doesn't affect functionality and is more of a code refactor we discussed with moving responsibility of telling what type of suggestion presentation we should use to the providers once they have determined that they will handle the current flag, i don't think it makes sense to rush through that refactor or to block the feature just because of it, I can make this update after the main PR is accepted if you agree and are ok with that and we don't find any other issues

hmhealey

Looks good. I'm not sure why the readme changed, so that would be nice to get reverted, but it's not a big deal

hmhealey · 2018-08-28T20:13:45Z

README.md

@@ -48,4 +48,4 @@ Get the Latest News:
 - **Email** - Subscribe to our [newsletter](http:https://mattermost.us11.list-manage.com/subscribe?u=6cdba22349ae374e188e7ab8e&id=2add1c8034) (1 or 2 per month)
 - **IRC** - Join us on #matterbridge (thanks to [matterircd](https://github.com/42wim/matterircd))

-Any other questions, mail us at [email protected]. We'd love to meet you!
+Any other questions, mail us at [email protected]. We'd love to meet you!


I'm not sure what's changed here since there's no new line changing here

i used that to trigger another build earlier because jenkins got stuck and wasn't picking up the latest changes, pushing this commit worked, it can be reverted, i just removed an empty line

done, reverted this change

sudheerDev

Thanks 👍 . Looks good. I just have few minor questions.

sudheerDev · 2018-08-28T20:39:48Z

components/suggestion/search_date_provider.jsx

+ return (
+ <DayPicker
+ onDayClick={this.handleDayClick}
+ showOutsideDays={true}


Nit picking here: We can disable all future days here with disabledDays={{after: new Date()}}

I tried that, and it only visually disabled the days, but you could still view and click on them. I think that's something we can address in a followup

Sure of course. This is a non-blocking thing. You are right just disabling them is not enough i think we need to use it for onClick to disable

i don't think it is even needed, let the user select what they want, it doesn't break anything, if they want to select a date in the future, sure, they just won't get any results

sudheerDev · 2018-08-28T20:51:28Z

components/suggestion/suggestion_date.jsx

+ SuggestionStore.removeSuggestionsChangedListener(this.props.suggestionId, this.handleSuggestionsChanged);
+ }
+
+ getContent() {


I see references of this in code but I don't follow it. How is this called?

We can remove that. It's copied from the SuggestionList component, but that only needs it because of some code that it shares with the SearchSuggestionList, but the SuggestionDate component is completely independent of those two

sudheerDev · 2018-08-28T20:55:23Z

components/suggestion/suggestion_date.jsx

+ this.setState(this.getStateFromStores());
+ }
+
+ handleOnBlur(event) {


Do we use this?

sudheerDev · 2018-08-28T20:57:14Z

components/suggestion/suggestion_date.jsx

+ static propTypes = {
+ suggestionId: PropTypes.string.isRequired,
+ location: PropTypes.string,
+ renderDividers: PropTypes.bool,


Same as above.

Some more holdovers from the SuggestionList

sudheerDev · 2018-08-28T20:57:56Z

components/suggestion/suggestion_date.jsx

+export default class SuggestionDate extends React.Component {
+ static propTypes = {
+ suggestionId: PropTypes.string.isRequired,
+ location: PropTypes.string,


I don't see any references to this. Do we use this prop?

mattermod · 2018-08-28T22:00:00Z

Spinmint test server destroyed

sudheerDev · 2018-08-28T22:03:40Z

components/suggestion/suggestion_box.jsx

+ <SuggestionDateComponent
+ suggestionId={this.suggestionId}
+ location={listStyle}
+ renderDividers={renderDividers}


@hmhealey can you remove these as well?

@sudheerDev i removed these

…component in SuggestionBox

mattermod · 2018-08-31T14:32:05Z

Spinmint test running for more than 7 days. This test server was terminated.

* initial implementation of UI for date based suggestions * cleaned up some unneeded code that was copied over, added logic to display the suggestion in a list or date type of a suggestion conditionally based on the provider that triggers handling for the current flag, added conditional triggering of the blur even on the suggestion box (for date presentation we don't want to trigger blur as soon as the suggestion box input loses focus because the daypicker suggestion may require multiple clicks to get to the needed selection, particularly when changing months before selecting a day) * cleaned up code to meet requirements of `make check-style` * Updating styling for datepicker * add function to retrieve browser time zone offset in minutes and pass it in seconds to the server when performing a search of posts * updated to latest version of redux * fixed search posts rhs test * fixed error reported from "make check-style" * removed new line at the end to trigger a new build, jenkins wasn't pulling in the latest commit * updated the search help tooltip text as requested in PR * reverted earlier unnecessary change * Remove unused code from SuggestionDate * removed unused properties passed in when initializing SuggestionDate component in SuggestionBox

dmitrysamuylovpharo added 4 commits August 15, 2018 13:52

initial implementation of UI for date based suggestions

4ff2d00

Merge branch 'master' into feature/search-after-before-on

018c26f

cleaned up code to meet requirements of make check-style

f872798

esethna assigned esethna and wiersgallak Aug 15, 2018

jwilander added the 1: PM Review Requires review by a product manager label Aug 16, 2018

dmitrysamuylovpharo mentioned this pull request Aug 16, 2018

Feature/search after before on mattermost/mattermost#9219

Merged

2 tasks

wiersgallak added the Setup Old Test Server Triggers the creation of a test server label Aug 16, 2018

wiersgallak requested a review from asaadmahmood August 16, 2018 13:39

Updating styling for datepicker

3ce6985

asaadmahmood removed the Setup Old Test Server Triggers the creation of a test server label Aug 17, 2018

mattermost deleted a comment from mattermod Aug 17, 2018

asaadmahmood added Setup Old Test Server Triggers the creation of a test server and removed Setup Old Test Server Triggers the creation of a test server labels Aug 17, 2018

mattermost deleted a comment from mattermod Aug 17, 2018

asaadmahmood added the Setup Old Test Server Triggers the creation of a test server label Aug 17, 2018

hmhealey removed the Setup Old Test Server Triggers the creation of a test server label Aug 17, 2018

mattermost deleted a comment from mattermod Aug 17, 2018

hmhealey added the Setup Old Test Server Triggers the creation of a test server label Aug 17, 2018

Merge branch 'master' into feature/search-after-before-on

49960fc

hmhealey removed the Setup Old Test Server Triggers the creation of a test server label Aug 17, 2018

esethna requested review from hmhealey and enahum August 28, 2018 19:48

esethna added the 2: Dev Review Requires review by a core commiter label Aug 28, 2018

esethna approved these changes Aug 28, 2018

View reviewed changes

esethna removed the 1: PM Review Requires review by a product manager label Aug 28, 2018

esethna requested review from sudheerDev and removed request for enahum August 28, 2018 19:59

updated the search help tooltip text as requested in PR

cec5527

hmhealey approved these changes Aug 28, 2018

View reviewed changes

reverted earlier unnecessary change

300345d

sudheerDev reviewed Aug 28, 2018

View reviewed changes

Remove unused code from SuggestionDate

f7855f0

hmhealey removed the Setup Old Test Server Triggers the creation of a test server label Aug 28, 2018

sudheerDev reviewed Aug 28, 2018

View reviewed changes

sudheerDev approved these changes Aug 28, 2018

View reviewed changes

removed unused properties passed in when initializing SuggestionDate …

3313452

…component in SuggestionBox

sudheerDev added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Aug 29, 2018

sudheerDev merged commit eb7be9b into mattermost:master Aug 29, 2018

amyblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation labels Aug 29, 2018

lindalumitchell added the Tests/Done Release tests have been written label Sep 5, 2018

amyblais added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Sep 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/search after before on #1574

Feature/search after before on #1574

dmitrysamuylovpharo commented Aug 15, 2018 •

edited

Loading

michaelgamble commented Aug 16, 2018

dmitrysamuylovpharo commented Aug 17, 2018 •

edited

Loading

esethna commented Aug 28, 2018

esethna commented Aug 28, 2018

dmitrysamuylovpharo commented Aug 28, 2018

hmhealey left a comment

hmhealey Aug 28, 2018

dmitrysamuylovpharo Aug 28, 2018

dmitrysamuylovpharo Aug 28, 2018

sudheerDev left a comment

sudheerDev Aug 28, 2018

hmhealey Aug 28, 2018

sudheerDev Aug 28, 2018

dmitrysamuylovpharo Aug 29, 2018

sudheerDev Aug 28, 2018

hmhealey Aug 28, 2018

sudheerDev Aug 28, 2018

sudheerDev Aug 28, 2018

hmhealey Aug 28, 2018

sudheerDev Aug 28, 2018

mattermod commented Aug 28, 2018

sudheerDev Aug 28, 2018

dmitrysamuylovpharo Aug 29, 2018

mattermod commented Aug 31, 2018

Feature/search after before on #1574

Feature/search after before on #1574

Conversation

dmitrysamuylovpharo commented Aug 15, 2018 • edited Loading

Summary

Ticket Link

Checklist

michaelgamble commented Aug 16, 2018

dmitrysamuylovpharo commented Aug 17, 2018 • edited Loading

esethna commented Aug 28, 2018

esethna commented Aug 28, 2018

dmitrysamuylovpharo commented Aug 28, 2018

hmhealey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudheerDev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattermod commented Aug 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattermod commented Aug 31, 2018

dmitrysamuylovpharo commented Aug 15, 2018 •

edited

Loading

dmitrysamuylovpharo commented Aug 17, 2018 •

edited

Loading